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 791292 - [review] lr/ugly-descriptions: try harder to sanitize garbage product info from udev hwdb
[review] lr/ugly-descriptions: try harder to sanitize garbage product info fr...
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: Lubomir Rintel
NetworkManager maintainer(s)
: 738458 (view as bug list)
Depends on:
Blocks: nm-review
 
 
Reported: 2017-12-06 02:21 UTC by zhangxianwei8
Modified: 2018-02-24 00:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
1024x768 resolution (416.53 KB, image/png)
2017-12-06 02:21 UTC, zhangxianwei8
  Details
1440x900 resolution (669.93 KB, image/png)
2017-12-06 02:24 UTC, zhangxianwei8
  Details
change Ethernet name displays from description to interface (1.48 KB, patch)
2017-12-06 03:11 UTC, zhangxianwei8
none Details | Review
Modified effect-1024x768.png (500.79 KB, image/png)
2017-12-06 03:13 UTC, zhangxianwei8
  Details
0001-applet-change-Ethernet-name-displays-from-description-to-interface.patch (6.67 KB, patch)
2017-12-07 11:46 UTC, zhangxianwei8
none Details | Review
screenshot-1 (498.69 KB, image/png)
2017-12-07 11:50 UTC, zhangxianwei8
  Details
screenshot-2 (495.78 KB, image/png)
2017-12-07 11:50 UTC, zhangxianwei8
  Details
Fixed screenshot (466.94 KB, image/png)
2018-02-23 14:48 UTC, Lubomir Rintel
  Details

Description zhangxianwei8 2017-12-06 02:21:54 UTC
Created attachment 365083 [details]
1024x768 resolution

In a low resolution case, the network card's information displays too wide, especially more than one network card. Looking at the attachment, it's not very friendly.

In a normal resolution case, it's not a little friendly too.

network-manager-applet: master
OS: Fedora 27 MATE
Comment 1 zhangxianwei8 2017-12-06 02:24:39 UTC
Created attachment 365084 [details]
1440x900 resolution

This is 1440x900 resolution screen shot.(normal resolution)
Comment 2 zhangxianwei8 2017-12-06 03:11:31 UTC
Created attachment 365085 [details] [review]
change Ethernet name displays from description to interface

[PATCH]:0001-applet-change-Ethernet-name-displays-from-description-to-interface.patch
Comment 3 zhangxianwei8 2017-12-06 03:13:18 UTC
Created attachment 365086 [details]
Modified effect-1024x768.png

Modified effect:Modified effect-1024x768.png
Comment 4 Thomas Haller 2017-12-06 15:05:21 UTC
this fix doesn't seem right.

First, nm_device_get_description() is supposed to give a pretty display name for the device. If it's too long, it should be fixed at https://cgit.freedesktop.org/NetworkManager/NetworkManager/tree/libnm/nm-device.c?id=cdbe1ad7151266c3ddf594050c8b706ffd9ed19b#n1440


Second, if libnm gives a crazy long name, then applet should do something sensible, like truncating the text. Probably truncating the string itself is not nice. It seems better if the GTK widget itself would restrict itself in size and truncate the string thereby (automatically).
Comment 5 zhangxianwei8 2017-12-07 11:46:13 UTC
Created attachment 365189 [details] [review]
0001-applet-change-Ethernet-name-displays-from-description-to-interface.patch

Yes,probably truncating the string is not a better method, because it could lose some usfull informations. Maybe using tooltip to display description is a good idea, when mouse moved on it?

I've already done it, see the attachments.
Comment 6 zhangxianwei8 2017-12-07 11:50:04 UTC
Created attachment 365190 [details]
screenshot-1

See the attachment taking effect
Comment 7 zhangxianwei8 2017-12-07 11:50:43 UTC
Created attachment 365191 [details]
screenshot-2

See the attachment taking effect
Comment 8 zhangxianwei8 2017-12-12 08:53:07 UTC
Could you take a little time to see it? Thanks:) @Thomas Haller
Comment 9 zhangxianwei8 2018-01-12 01:52:30 UTC
This modification is OK?
Comment 10 Beniamino Galvani 2018-01-12 10:21:41 UTC
Especially for some kind of devices (modems, wifi adapters) I think the description is generally more clear for the user than the interface name.

Can we keep displaying the description and force a maximum width of the menu item? In case it's truncated, a tooltip could be used to display the full text. Maybe, the tooltip could also include the interface name, which is useful when there are multiple devices having the same description.
Comment 11 zhangxianwei8 2018-01-15 03:26:59 UTC
How can we force a maximum width of the menu item?
Comment 12 Thomas Haller 2018-02-22 16:01:16 UTC
*** Bug 738458 has been marked as a duplicate of this bug. ***
Comment 13 zhangxianwei8 2018-02-23 01:03:39 UTC
Hi, how about your opinion? @@Thomas Haller
Comment 14 Thomas Haller 2018-02-23 13:34:14 UTC
(In reply to zhangxianwei8 from comment #13)
> Hi, how about your opinion? @@Thomas Haller

My opinion is still comment 4 :)

nm_device_get_description() is maybe a bad name, but it really is supposed to be the pretty name of the device. If it doesn't fulfil that, it needs to be fixed. The GUI should show the pretty name.

Seems Lubomir is working to improve nm_device_get_description() (the "First" part).
Comment 15 Lubomir Rintel 2018-02-23 14:48:59 UTC
Created attachment 368829 [details]
Fixed screenshot

Ready for review: https://cgit.freedesktop.org/NetworkManager/NetworkManager/log/?h=lr/ugly-descriptions
Comment 16 Beniamino Galvani 2018-02-23 16:21:00 UTC
> libnm/utils: ignore stuff in parentheses for vendor/product fixups

+               if (*p == ')')
+                       in_paren = TRUE;

This should be FALSE?

> libnm/utils: drop part after a dash in product name

+       /* Chop off everything after a '-'. */
+       for (p = desc_full; *p; p++) {
+               if ((p[0] == ' ' && p[1] == '-') && p[2] == ' ') {

Extra parentheses   ^                          ^

The rest LGTM.
Comment 17 Lubomir Rintel 2018-02-23 18:48:23 UTC
Thanks. Merged with fixes for the above.
Comment 18 zhangxianwei8 2018-02-23 19:56:11 UTC
Well done. But I think there should add the info include the interface name, which is useful when there are multiple devices having the same description.
Comment 19 Thomas Haller 2018-02-23 20:10:17 UTC
(In reply to zhangxianwei8 from comment #18)
> Well done. But I think there should add the info include the interface name,
> which is useful when there are multiple devices having the same description.

I tend to agree.

I still would like to see that overly long texts are truncated in the applet GUI -- although, now the texts should not be too long anymore.
Comment 20 zhangxianwei8 2018-02-24 00:58:58 UTC
Why not use tooltip to display more information?