After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 723378 - error messages not displaying when updating a printer
error messages not displaying when updating a printer
Status: RESOLVED OBSOLETE
Product: gnome-control-center
Classification: Core
Component: Printers
3.8.x
Other Linux
: Normal minor
: ---
Assigned To: Marek Kašík
Control-Center Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-01-31 16:13 UTC by Patrick Laxton
Modified: 2021-06-09 16:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
replaced deprecated widgets (40.46 KB, patch)
2015-04-23 10:21 UTC, mhatina
none Details | Review
add function for showing of warnings (59.50 KB, patch)
2015-04-23 10:22 UTC, mhatina
rejected Details | Review
add function for validation of printer name (1.42 KB, patch)
2015-04-23 10:22 UTC, mhatina
needs-work Details | Review
show warning when renaming failed (1.74 KB, patch)
2015-04-23 10:23 UTC, mhatina
rejected Details | Review
Screenshot of warning (31.53 KB, image/png)
2015-04-23 10:24 UTC, mhatina
  Details
replaced deprecated widgets (14.24 KB, patch)
2015-04-28 09:19 UTC, mhatina
none Details | Review
replaced some deprecated widgets (3.60 KB, patch)
2015-04-29 12:31 UTC, mhatina
none Details | Review
replaced deprecated widgets (3.53 KB, patch)
2015-04-30 12:17 UTC, mhatina
committed Details | Review
printers: handle non-ASCII printer-names (1014 bytes, patch)
2015-08-27 20:56 UTC, Felipe Borges
none Details | Review
printers: handle non-ASCII printer-names (1014 bytes, patch)
2015-08-27 21:17 UTC, Felipe Borges
none Details | Review

Description Patrick Laxton 2014-01-31 16:13:42 UTC
I installed a network printer on my laptop using CUPS, which allowed me to enter a name and location with white spaces and French accents (é, â, ...).

When I tried to rename the printer using gnome-control-center (because CUPS called it something like "HP_HP_serial_number_..."), it just kept on returning back to the CUPS name, without telling me anything.

I launched gnome-control-center with root privileges (sudo /usr/bin/gnome-control-center), and I got this message in the shell when the name was returned to "HP_HP_..." :
(gnome-control-center:9921): printers-cc-panel-WARNING **: "La seule vraie imprimante" is not a valid printer name.

My problem here is that I had to look at error logs to see such a message, no window told me, thus I assumed it was a privilege error.

I got a second one, then :
gnome-control-center:9990): printers-cc-panel-WARNING **: "derrière toi" is not a valid location.

I assume gnome-control-center doesn't like white spaces in names, whereas CUPS do (and thus, Linux itself must like white spaces too).

So I think there are two feature requests behind that report :
 - show the user these "printers-cc-panel-WARNING" in a user-friendly manner (like, in a window or in the message tray, I don't know);
 - let the user put white spaces in printers names, locations, metadatas, ... (let's say UTF-8 characters, why not?)
Comment 1 mhatina 2015-04-23 10:21:27 UTC
Created attachment 302206 [details] [review]
replaced deprecated widgets
Comment 2 mhatina 2015-04-23 10:22:12 UTC
Created attachment 302207 [details] [review]
add function for showing of warnings
Comment 3 mhatina 2015-04-23 10:22:49 UTC
Created attachment 302208 [details] [review]
add function for validation of printer name
Comment 4 mhatina 2015-04-23 10:23:13 UTC
Created attachment 302209 [details] [review]
show warning when renaming failed
Comment 5 mhatina 2015-04-23 10:24:35 UTC
Created attachment 302210 [details]
Screenshot of warning
Comment 6 mhatina 2015-04-23 10:26:26 UTC
I have submitted few patches for showing warning when renaming of printer failed. There is also screenshot of how it looks like.
Comment 7 Bastien Nocera 2015-04-23 12:12:31 UTC
Review of attachment 302206 [details] [review]:

There's a lot of churn in there, including adding and removing properties, changed lines because of glade moving them.

You might want to re-do this using a text editor instead, or carefully reviewing which lines changed before adding them to the commit.

"git add -p" can prove useful here :)

::: panels/printers/printers.ui
@@ -7,3 @@
     <property name="orientation">vertical</property>
     <property name="spacing">10</property>
-    <property name="margin-start">6</property>

The only reason this chunk is in the patch is because it got moved a couple of lines earlier. It shouldn't be in the patch.

@@ +16,2 @@
         <property name="visible">True</property>
+        <property name="can_focus">False</property>

This shouldn't be in the patch, for example.

@@ -65,3 @@
                     <property name="use_underline">True</property>
                     <property name="icon_name">list-remove-symbolic</property>
-                    <property name="label" translatable="yes">Remove Printer</property>

This just moved.

@@ +78,3 @@
                 </child>
+                <style>
+                  <class name="inline-toolbar"/>

This just moved.
Comment 8 Bastien Nocera 2015-04-23 12:18:55 UTC
I don't think this is the UI we want. If spaces aren't allowed, you shouldn't be able to type spaces in the name, ditto for non-ASCII characters.

But I find it bizarre that the users aren't allowed to name printers with non-ASCII characters, or spaces. It's an ID then, isn't it?

Better would be for the panel to change the user-visible name, and save it, and deduce the ID from the name. We do something similar to transform machine's PrettyNames to hostnames:
"Bãstien's printer... Foo-bar" would become "bastiens-printer-foo-bar" as an ID.

That code lives in shell/hostname-helper.c in gnome-control-center. You'll just need to figure out a way to store the user's printer names (and make sure that the GTK+ printer panel uses them too).
Comment 9 Bastien Nocera 2015-04-23 12:20:35 UTC
Review of attachment 302208 [details] [review]:

::: panels/printers/cc-printers-panel.c
@@ +1869,3 @@
 }
 
+// Function taken from cups-pk-helper

We don't use C++-style comments in GNOME. If you're going to copy/paste functions from other sources, please include a full URL to the source, as well as copyright information for it.
Comment 10 Bastien Nocera 2015-04-23 12:21:18 UTC
Review of attachment 302209 [details] [review]:

Rejected, as per earlier comment. Showing a warning isn't the right way to solve this.
Comment 11 Bastien Nocera 2015-04-23 12:23:58 UTC
It's possible that showing a warning is only way we can technically solve it, but this would likely mean that changing the name of the printer would popup a dialogue, so that the warning can be shown in the dialogue, and the "OK" button can be disabled if forbidden characters are used.

We can talk to the designers once we've ruled out whether having users allowed to use arbitrary names is possible or not.
Comment 12 Marek Kašík 2015-04-23 13:25:09 UTC
(In reply to Bastien Nocera from comment #8)
> I don't think this is the UI we want. If spaces aren't allowed, you
> shouldn't be able to type spaces in the name, ditto for non-ASCII characters.

I actually like the idea of showing the warning. But maybe we could modify CcEditableEntry class to reject those characters during typing.


> But I find it bizarre that the users aren't allowed to name printers with
> non-ASCII characters, or spaces. It's an ID then, isn't it?

It is the way how CUPS implements printer-name. It doesn't allow non-ASCII characters or characters rejected by the patch.
It serves as an ID too but still it is a name.


> Better would be for the panel to change the user-visible name, and save it,
> and deduce the ID from the name. We do something similar to transform
> machine's PrettyNames to hostnames:
> "Bãstien's printer... Foo-bar" would become "bastiens-printer-foo-bar" as an
> ID.
> 
> That code lives in shell/hostname-helper.c in gnome-control-center. You'll
> just need to figure out a way to store the user's printer names (and make
> sure that the GTK+ printer panel uses them too).

I don't think that changing the handling of printer names in all libraries/applications just because we don't like CUPS' implementation is a good idea. We should just set what system allows us because that's what applications use (and we can not change them all). Making gnome-control-center a special case would lead to inconsistency.
Comment 13 Bastien Nocera 2015-04-23 13:31:36 UTC
(In reply to Marek Kašík from comment #12)
> I don't think that changing the handling of printer names in all
> libraries/applications just because we don't like CUPS' implementation is a
> good idea.

I think that not liking it because it's awful is fine actually...
Comment 14 mhatina 2015-04-28 09:19:49 UTC
Created attachment 302505 [details] [review]
replaced deprecated widgets

Fixed ui patch.
Comment 15 Allan Day 2015-04-28 10:18:45 UTC
I largely agree with Bastien's view on this bug. Reverting the rename and showing a warning is a poor user experience - it forces the user to go back and do the rename again. Also, you haven't provided any guidance on what the naming policy is, so they have no clue why the new name was rejected.

Possible ways to deal with this (all of which have already been mentioned by Bastien):

 1. Change the naming policy to be more permissive.
 2. When the user enters an invalid name, convert it to one that is allowed, such as replacing spaces with dashes.
 3. When the name is being edited, show a "Rename" button. When an invalid name is entered, make the button insensitive and show a string that explains why the new name isn't allowed (for example: "Printer names cannot include spaces.")

3 could be done within the panel, or you could change printer renaming to be done in a dialog instead.
Comment 16 mhatina 2015-04-28 10:40:24 UTC
Thank you.

option 1. -- We can't do that, see comment #12.

option 2. -- I don't think it will be as good as it looks like on first sight, there will be problem with diacritics (in slovak "moja tlačiareň" would be "moja-tla-iare-")

option 3. -- Could you show me how it should look like (provide some mockup) or provide link on something similar that is already in control-center?
Comment 17 Bastien Nocera 2015-04-28 11:13:23 UTC
(In reply to mhatina from comment #16)
> Thank you.
> 
> option 1. -- We can't do that, see comment #12.

I don't think that comment #12 is accurate. You could do most of the work inside the cups-pk-helper and have that be your "CUPS ID to User-facing name" cache.
Comment 18 Marek Kašík 2015-04-29 08:06:20 UTC
Review of attachment 302505 [details] [review]:

Hi Martin,

thank you for the patch. Please remove the replacement of the GtkTable by GtkGrid for now, I've tested this more yesterday and GtkGrid is not able to bring in the same behaviour as GtkTable had. I haven't found a way how ellipsize the printer name and location as before. I'll look at this more later but not now, it looks like a bug in handling of the '*expand' and '*fill' properties in Gtk+ to me.
Please use 'printers: Replace deprecated widgets' in the patch's subject and 'Replace some deprecated widgets.' in the description.

::: panels/printers/printers.ui
@@ +527,3 @@
             </child>
             <child>
+              <object class="GtkBox" id="vbox7">

This is missing the orientation specification.
Comment 19 Marek Kašík 2015-04-29 11:06:39 UTC
(In reply to Bastien Nocera from comment #17)
> (In reply to mhatina from comment #16)
> > Thank you.
> > 
> > option 1. -- We can't do that, see comment #12.
> 
> I don't think that comment #12 is accurate. You could do most of the work
> inside the cups-pk-helper and have that be your "CUPS ID to User-facing
> name" cache.

I don't plan to add such cache to cups-pk-helper.
Comment 20 mhatina 2015-04-29 12:31:44 UTC
Created attachment 302561 [details] [review]
replaced some deprecated widgets

Fixed. GtkTable remains intact, vbox7 has correct orientation now.
Comment 21 Marek Kašík 2015-04-29 13:45:25 UTC
Review of attachment 302561 [details] [review]:

Thank you for the modification. Please use 'printers: Replace deprecated widgets' in the patch's subject and 'Replace some deprecated widgets.' in the description.

::: panels/printers/printers.ui
@@ +36,3 @@
               <packing>
+                <property name="expand">True</property>
+                <property name="fill">True</property>

You don't need to set the 'fill' property to 'True', it is default value.
Comment 22 mhatina 2015-04-30 12:17:01 UTC
Created attachment 302643 [details] [review]
replaced deprecated widgets

Fixed.
Comment 23 Marek Kašík 2015-04-30 12:28:17 UTC
Review of attachment 302643 [details] [review]:

Thank you for the modifications. Let's push it to master after gnome-3-16 will be created.
Comment 24 Felipe Borges 2015-08-27 20:56:19 UTC
Created attachment 310131 [details] [review]
printers: handle non-ASCII printer-names

Serialize printer names instead of having an ASCII-only policy.
Comment 25 Felipe Borges 2015-08-27 20:58:30 UTC
I think we can have "option 2" as proposed by Allan using g_str_to_ascii (defaulting for the current locale) and replacing white-spaces with hyphens.
Comment 26 Felipe Borges 2015-08-27 21:17:05 UTC
Created attachment 310137 [details] [review]
printers: handle non-ASCII printer-names

Serialize printer names instead of having an ASCII-only policy.
Comment 27 André Klapper 2021-06-09 16:12:35 UTC
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org.
As part of that, we are mass-closing older open tickets in bugzilla.gnome.org
which have not seen updates for a longer time (resources are unfortunately
quite limited so not every ticket can get handled).

If you can still reproduce the situation described in this ticket in a recent
and supported software version, then please follow
  https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines
and create a new bug report at
  https://gitlab.gnome.org/GNOME/gnome-control-center/-/issues/

Thank you for your understanding and your help.