GNOME Bugzilla – Bug 790361
After renaming a printer, the printer sometimes appears twice in the list: with old and new name
Last modified: 2018-02-05 13:18:51 UTC
Steps to reproduce: * Add a printer * Rename it using the "Printer Details" window What happens now is that after renaming, the entire list briefly flashes. Sometimes, after the flash, the printer is still shown under its old name. Then the list flashes again, and now the same printer is shown twice: Under the old name, and under the new name. Switching to another page in the control center and back to the printer page gets rid of the "ghost" printer. This is with CUPS 2.2.6 on Debian testing amd64.
Felipe, please have a look at this. Thanks.
Created attachment 364128 [details] [review] printers: Prevent renamed printer from been shown twice When a printer is renamed it might happen that both its original name and the new name are presented as its own entries for a second. This lasts until the renaming operation finishes and the printer list gets updated. This hotfix solution stores a printer-old-name in the PpDetailsDialog object, which gets extracted into the PpPrinterEntry object. This way the main panel can blacklist the renamed printer while updating the list. This is not a definitive fix for the issue, but a temporary fix until we land some backend refactorings. This is intented as a minimal possible fix to be backported in downstreams.
Review of attachment 364128 [details] [review]: It seems that I've mystified you when we talked about this. We need a little different solution a signal-based one. Lets add signals "printer-renamed" to both PpPrinterEntry and PpDetailsDialog (this would actually mean that the printer is being renamed - use a better name of the signal if you want). This would deliver the new name with it to the CcPrintersPanel. There we need to get both the new name from the signal and the old name from the entry and store them. Once we actualize printers list after we receive the signal from an entry we have to check whether the new printer is already available and don't show the old one in that case otherwise still show the old one.
Created attachment 365115 [details] [review] printers: Don't show duplicates while renaming printer During the time it took to rename a printer asynchronously, we had cases where two entries were shown for the same printer: one with the old name and another with a new name. Now we signal from DetailsDialog to the given PrinterEntry which passes it along to the main panel object. The CcPrintersPanel object blacklists the renamed printer old name.
Review of attachment 365115 [details] [review]: Thank you for the patch. It still needs some work though :). ::: panels/printers/cc-printers-panel.c @@ +734,3 @@ +static void +on_printer_renamed (PpPrinterEntry *printer_entry, + gchar *old_name, This should be actually new_name. Get the old_name from the printer_entry directly. @@ +742,3 @@ + priv = PRINTERS_PANEL_PRIVATE (self); + + priv->renamed_printer_name = g_strdup (old_name); Store old name and also new name. You will need it. @@ +855,3 @@ continue; + if (g_strcmp0 (priv->dests[i].name, priv->renamed_printer_name) == 0) You have to check whether the new printer is already present. So a small loop checking all dests should be placed somewhere before this with a "new_printer_available" boolean as a result. And don't show the old one only if the new one is already there: if (new_printer_available && g_strcmp0 (priv->dests[i].name, priv->renamed_printer_old_name) == 0) continue; It happens here that the printer disappears until I somehow "refresh" the panel. ::: panels/printers/pp-details-dialog.c @@ +67,3 @@ GtkDialogClass parent_class; + + void (*printer_renamed) (PpDetailsDialog *details_dialog); Shouldn't there be also the "const gchar *new_name" here? @@ +112,3 @@ PpPrinter *printer = pp_printer_new (self->printer_name); + g_signal_emit_by_name (self, "printer-renamed", self->printer_name); Send the "new_name" here instead of the old name. ::: panels/printers/pp-printer-entry.c @@ +94,3 @@ void (*printer_changed) (PpPrinterEntry *printer_entry); void (*printer_delete) (PpPrinterEntry *printer_entry); + void (*printer_renamed) (PpPrinterEntry *printer_entry); I think that the new_name should be listed here too. @@ +395,3 @@ +static void +printer_renamed_cb (PpDetailsDialog *dialog, + gchar *old_name, This should be named "new_name".
Created attachment 365735 [details] [review] printers: Don't show duplicates while renaming printer During the time it took to rename a printer asynchronously, we had cases where two entries were shown for the same printer: one with the old name and another with a new name. Now we signal from DetailsDialog to the given PrinterEntry which passes it along to the main panel object. The CcPrintersPanel object blacklists the renamed printer old name.
Review of attachment 365735 [details] [review]: Thank you for the patch. Push it to master branch please.
Attachment 365735 [details] pushed as 3456546 - printers: Don't show duplicates while renaming printer Thanks for your review!