Bug 636523

+++ This bug was initially created as a clone of Bug #625502 +++

abrt version: 1.1.13
architecture: i686
cmdline: python /usr/share/system-config-printer/system-config-printer.py
component: system-config-printer
executable: /usr/share/system-config-printer/system-config-printer.py
kernel: 2.6.33.6-147.2.4.fc13.i686
package: system-config-printer-1.2.3-3.fc13
reason: system-config-printer.py:3151:rename_printer:AttributeError: 'NoneType' object has no attribute 'setAccepting'
release: Fedora release 13 (Goddard)
time: 1282235009
uid: 500

backtrace
-----
system-config-printer.py:3151:rename_printer:AttributeError: 'NoneType' object has no attribute 'setAccepting'

Traceback (most recent call last):
  File "/usr/share/system-config-printer/system-config-printer.py", line 3085, in printer_name_edited
    self.rename_printer (name, newname)
  File "/usr/share/system-config-printer/system-config-printer.py", line 3151, in rename_printer
    self.printer.setAccepting (True)
AttributeError: 'NoneType' object has no attribute 'setAccepting'

Local variables in innermost frame:
new_name: 'c6'
self: <__main__.GUI instance at 0x8867bcc>
rejecting: False
old_name: u'C6'

--- Additional comment from liblit on 2010-08-19 18:24:04 CEST ---

Created an attachment (id=439729)
File: backtrace

--- Additional comment from jpopelka on 2010-08-20 13:36:00 CEST ---

Not sure how this could happen. 

The code preceding the crash is:

        rejecting = self.printer.rejecting
        if not rejecting:
            try:
                self.printer.setAccepting (False)
                if not self.is_rename_possible (old_name):
                    [...]
                    return
            except cups.IPPError, (e, msg):
                [...]
                return

        if self.duplicate_printer (new_name):
            [...]
            return

        # Restore rejecting state.
        if not rejecting:
            try:
                self.printer.setAccepting (True)  <- here the self.printer suddenly has None value.

The only possibility is that self.duplicate_printer() changed the self.printer value to None. But investigating the self.duplicate_printer() it doesn't look so.
NEEDS MORE DIGGING.

--- Additional comment from twaugh on 2010-08-20 14:00:35 CEST ---

I think it must be this:

During save_printer, we fall into the "printer was deleted in the mean time" part and so call populateList().  At the end of that function we have:

        if (self.printer != None and
            self.printer.name not in self.printers.keys ()):
            # The printer we're editing has been deleted.
            self.PrinterPropertiesDialog.response (gtk.RESPONSE_CANCEL)

...which does this:

        if ((response == gtk.RESPONSE_OK and not failed) or
            response == gtk.RESPONSE_CANCEL):
            self.printer = None
            dialog.hide ()

--- Additional comment from jpopelka on 2010-09-21 16:31:30 CEST ---

Steps to reproduce:
1) rename printer queue where the new name differs
    only in size (lower/upper case) of some characters.
    E.g. C6 -> c6 (comment #0)

When s-c-p renames queue, it first makes a new one with the new name,
copies attributes and deletes the old one.
Problem is that CUPS-Add-Modify-Printer operation is case-insensitive in sense
that when it discovers that adding printer already exists (ignoring case) it modifies the existing one.
For looking up/comparing strcasecmp() is used, see. scheduler/printers.c:compare_printers().

In our case it means that when s-c-p tries to add a new printer in save_printer(), there's actually no adding
happening because there's already one with the same name (ignoring case) existing.
And at the end of save_printer() we discover that the printer with new name (case sensitive) doesn't exist
and populateList() does self.printer=None.

--- Additional comment from jpopelka on 2010-09-21 16:56:03 CEST ---

Created attachment 448714 [details]
double-renaming

Possibilities
A) Do not rename when the new name differs only in size
   (lower/upper case) of some characters.
    def rename_printer (self, old_name, new_name):
-        if old_name == new_name:
+        if old_name.lower() == new_name.lower():
            return

B) Double-rename (old_name->random_name, random_name->new_name)
   (see attached patch)

C) Some dialog with warning

--- Additional comment from twaugh on 2010-09-22 12:16:13 CEST ---

I think A is best.  Let's follow the example of how cupsd behaves.

--- Additional comment from jpopelka on 2010-09-22 15:10:37 CEST ---

http://git.fedorahosted.org/git/?p=system-config-printer.git;a=commitdiff;h=332fdc86be253a3b91720f64685ed2507b308184