Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#3385 closed defect (fixed)

'dialog-cancel' icon has not the correct size when inside a gtk entry

Reported by: erikos Owned by: erikos
Priority: Unspecified by Maintainer Milestone:
Component: Sugar Version: Git as of bugdate
Severity: Major Keywords: 12.1.0, screenshot
Cc: garycmartin, humitos Distribution/OS: OLPC
Bug Status: Assigned

Description (last modified by erikos)

Steps to reproduce:

  • type something into the search field in the Home view

---> the 'dialog-cancel' icon to clear the field is too small

  • open Browse/Read and search a string in the page

---> the 'dialog-cancel' icon to clear the field is too big

Attachments (6)

browse-search.jpeg (215.4 KB) - added by humitos 7 years ago.
13.png (47.8 KB) - added by humitos 7 years ago.
Same error on Get Books Gtk3 port
home_72.png (22.7 KB) - added by erikos 7 years ago.
home 'sugar-emulator -i 1200x900' scaling is 72
home_100.png (30.3 KB) - added by erikos 7 years ago.
home 'sugar-emulator -d 200 -i 1200x900 -s 100' scaling is 100, XO setting
browse_72.png (39.0 KB) - added by erikos 7 years ago.
browse 'sugar-emulator -i 1200x900' scaling is 72
browse_100.png (53.8 KB) - added by erikos 7 years ago.
browse 'sugar-emulator -d 200 -i 1200x900 -s 100' scaling is 100, XO setting

Download all attachments as: .zip

Change History (28)

comment:1 Changed 7 years ago by erikos

  • Keywords 12.1.0 added

Still the case in os8. Seen as well issues with icons in Fedora beta (e.g. reload icon in url entry which is drawn to low).

Changed 7 years ago by humitos

comment:2 Changed 7 years ago by humitos

I think this is not related with browser. In fact, I think that it's related with sugar3.graphics.iconentry.IconEntry class because Browse just used it.

This is the relevant part of IconEntry class:

def show_clear_button(self):
    if not self._clear_shown:
        self.set_icon_from_name(ICON_ENTRY_SECONDARY,
                                'dialog-cancel')
        self._clear_shown = True

comment:3 Changed 7 years ago by erikos

  • Bug Status changed from Unconfirmed to Assigned
  • Cc garycmartin added
  • Component changed from Browse to sugar-artwork
  • Owner changed from erikos to benzea

This is as well an issue in Read.

I changed the icon from 'dialog-cancel' to 'system-search' in http://git.sugarlabs.org/sugar-toolkit-gtk3/sugar-toolkit-gtk3/blobs/master/src/sugar3/graphics/iconentry.py#line76 and the icon size is correct. So the issue seem to be in the icon itself.

Another example of this icon behaving strange is the serach in the home view. The icon is too small there.

comment:4 Changed 7 years ago by erikos

I remember that we did an dialog-cancel icon for Browse at one point, as far as I remember because we did not get the sizing in the entry right: http://git.sugarlabs.org/browse/mainline/blobs/master/icons/browse-dialog-cancel.svg

'dialog-ok' has the same issue. Both are larger than 'system-search' for example and in the entry they probably can not use the full size and scale badly (too small or too big).

Gary, any suggestions from your side?

comment:5 Changed 7 years ago by erikos

  • Description modified (diff)
  • Summary changed from Browse edit: in document search's stop icon is too big to 'dialog-cancel' icon has not the correct size when inside a gtk entry

comment:6 Changed 7 years ago by erikos

  • Owner changed from benzea to erikos
  • Status changed from new to assigned

comment:7 Changed 7 years ago by erikos

Ok, two test results:

Put the 'system-search' icon into the iconentry secondary icon which is used in the sugar home view search entry. The icon has the correct sizing. Put the 'system-search' icon as the icon for the view source dialog.

src/jarabe/view/viewsource.py:409:        stop = ToolButton(icon_name='dialog-cancel')

The result is that the icon has the correct sizing as well.

comment:8 Changed 7 years ago by humitos

  • Cc humitos added

comment:9 Changed 7 years ago by erikos

Carlos thinks this is an upstream issue: https://bugzilla.gnome.org/show_bug.cgi?id=677567

comment:10 Changed 7 years ago by garnacho

Yes, It's indeed that issue. I've tried the patch on sugar-emulator and the icon is displayed at the right size

comment:11 Changed 7 years ago by erikos

Side note about how to test that:

Add repository to the modules (e.g. config/modulesets/glucose-external.modules). This takes care of setting up the environment (e.g. PKG_CONFIG_PATH).

This adds GTK+ with using the master at the "3.4.3" tag (currently what is in F17).

+  <autotools id="gtk+">
+    <branch repo="git.gnome.org"
+            tag="3.4.3"/>
+    <dependencies>
+      <dep package="atk"/>
+      <dep package="glib"/>
+      <dep package="cairo"/>
+      <dep package="pango"/>
+      <dep package="gdk-pixbuf"/>
+      <dep package="gtk-doc"/>
+      <dep package="gobject-introspection"/>
+    </dependencies>
+    <suggests>
+      <dep package="shared-mime-info"/>
+    </suggests>
+  </autotools>

Then you can build it like: ./sugar-jhbuild buildone gtk+

You can apply now the patches using 'git bz' [1][2], the one from Carlos like "git bz apply bugzilla.gnome.org:677567"

[1] http://git.fishsoup.net/man/git-bz.html
[2] http://www.damnpeople.fr/2012/03/18/an-introduction-to-git-bz-or-how-to-make-your-life-simplier-when-working-with-git-and-bugzilla/

comment:12 Changed 7 years ago by erikos

Hmm, I did apply your patch, but the issue looks as before. Carlos, where did you verify it? In the search field in Browse?

comment:13 Changed 7 years ago by humitos

  • Keywords screenshot added

Just a comment: I'm having the same issue in the Get Books porting. Screenshot attached

Changed 7 years ago by humitos

Same error on Get Books Gtk3 port

comment:14 follow-up: Changed 7 years ago by erikos

Carlos, actually had an environmental issue, so false alarm about final success, he will investigate further.

comment:15 in reply to: ↑ 14 ; follow-up: Changed 7 years ago by garnacho

Replying to erikos:

Carlos, actually had an environmental issue, so false alarm about final success, he will investigate further.

I had one indeed, also badly failed to notice that set_icon_from_name() is not set_icon_from_icon_name() :), IconEntry ultimately uses GtkEntry.set_icon_from_pixbuf(), which codepath doesn't handle resize-to-fit, I filed a bug/patch to gtk+ upstream:

https://bugzilla.gnome.org/show_bug.cgi?id=678087

a short term fix until the patch is applied could be re-scaling the pixbuf before:
http://git.sugarlabs.org/sugar-toolkit-gtk3/sugar-toolkit-gtk3/blobs/master/src/sugar3/graphics/iconentry.py#line60
Should I do a patch for that?

comment:16 Changed 7 years ago by garnacho

Forgot to mention... the other opened gnome bug is valid, but not quite related to this bug in the end... it could be hit if sugar-artwork iconset gets to use symbolic icons as handled by gtk+:
http://developer.gnome.org/gtk3/3.2/GtkIconTheme.html#gtk-icon-info-load-symbolic

comment:17 in reply to: ↑ 15 Changed 7 years ago by erikos

Replying to garnacho:

Replying to erikos:

Carlos, actually had an environmental issue, so false alarm about final success, he will investigate further.

I had one indeed, also badly failed to notice that set_icon_from_name() is not set_icon_from_icon_name() :), IconEntry ultimately uses GtkEntry.set_icon_from_pixbuf(), which codepath doesn't handle resize-to-fit, I filed a bug/patch to gtk+ upstream:

https://bugzilla.gnome.org/show_bug.cgi?id=678087

a short term fix until the patch is applied could be re-scaling the pixbuf before:
http://git.sugarlabs.org/sugar-toolkit-gtk3/sugar-toolkit-gtk3/blobs/master/src/sugar3/graphics/iconentry.py#line60
Should I do a patch for that?

Yes, if you know already what to do, please go ahead, thanks.

Changed 7 years ago by erikos

home 'sugar-emulator -i 1200x900' scaling is 72

Changed 7 years ago by erikos

home 'sugar-emulator -d 200 -i 1200x900 -s 100' scaling is 100, XO setting

Changed 7 years ago by erikos

browse 'sugar-emulator -i 1200x900' scaling is 72

Changed 7 years ago by erikos

browse 'sugar-emulator -d 200 -i 1200x900 -s 100' scaling is 100, XO setting

comment:18 Changed 7 years ago by erikos

Ok, so what is interesting is that this issue is a regression from previous behavior in GTK+ 2. You can see this if you compare Log (GTK+ 2 activity) and Browse (GTK+ 3 activity), I printed out the pixbuf sizes in the iconentry and the following is the case:

Log: When the 'dialog-cancel' icon is added to the entry the pixbuf that gets created from the icon has the size 55. The 'system-search' icon that gets added has a pixbuf size of 22. Both are the actual definitions in the svg file. They both display at the correct size in the entry. The 'dialog-cancel' pixbuf must be changed accordingly.

Browse: The same icons are added to the entry in Browse. The 'system-search' icon already has the correct size. But the 'dialog-cancel' does not get adjusted accordingly in the pixbuf. You can do that with scaling as Carlos suggested as a workaround.

comment:19 Changed 7 years ago by erikos

The second issue I still see (even when doing the scaling either in the iconentry.py or using Carlos' patch) is that when I run the sugar-emulator at scaling factor 72 the icon has not the right positioning. See the "*_72.png"-screenshots from above.

comment:20 Changed 6 years ago by manuq

  • Resolution set to fixed
  • Status changed from assigned to closed

Let's keep track of the current issue in #3569 .

comment:21 Changed 6 years ago by dnarvaez

  • Component changed from sugar-artwork to Sugar

comment:22 Changed 6 years ago by dnarvaez

  • Milestone 0.96 deleted

Milestone 0.96 deleted

Note: See TracTickets for help on using tickets.