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 769533 - Use GTK 3.14 rtl icon support
Use GTK 3.14 rtl icon support
Status: RESOLVED FIXED
Product: geary
Classification: Other
Component: general
unspecified
Other All
: Normal normal
: 0.12.0
Assigned To: Geary Maintainers
Geary Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-08-05 04:29 UTC by Jeremy Bicha
Modified: 2016-08-06 01:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Use GTK 3.14 rtl icon support (11.17 KB, patch)
2016-08-05 04:29 UTC, Jeremy Bicha
none Details | Review
Fix typo in Bosnian translation (808 bytes, patch)
2016-08-05 04:30 UTC, Jeremy Bicha
none Details | Review

Description Jeremy Bicha 2016-08-05 04:29:08 UTC
Here's a completely untested patch for this
Comment 1 Jeremy Bicha 2016-08-05 04:29:11 UTC
Created attachment 332757 [details] [review]
Use GTK 3.14 rtl icon support
Comment 2 Jeremy Bicha 2016-08-05 04:30:35 UTC
Created attachment 332758 [details] [review]
Fix typo in Bosnian translation
Comment 3 Michael Gratton 2016-08-05 13:55:46 UTC
Hey Jeremy,

(In reply to Jeremy Bicha from comment #1)
> Created attachment 332757 [details] [review] [review]
> Use GTK 3.14 rtl icon support

Thanks for the patch! Turns out that the icons were already correctly named, so under 3.20 simply removing the rtl tests seemed to work fine. I'd need to check that was already the case under 3.14, though. There were also a couple of missing commas that valac complained about, too.

Are you happy for me to commit the modified patch on your behalf, or would you like to re-spin it yourself?

(In reply to Jeremy Bicha from comment #2)
> Created attachment 332758 [details] [review] [review]
> Fix typo in Bosnian translation

What's the etiquette here, do you know? Isn't it considered bad form for maintainers to mess around in translation team's files?
Comment 4 Jeremy Bicha 2016-08-05 14:07:50 UTC
(In reply to Michael Gratton from comment #3)
> Thanks for the patch! Turns out that the icons were already correctly named,
> so under 3.20 simply removing the rtl tests seemed to work fine. I'd need to
> check that was already the case under 3.14, though. There were also a couple
> of missing commas that valac complained about, too.

That's interesting if the old icon names work, but the pattern used in adwaita-icon-theme has rtl as a (final) suffix so I think it would still make sense to rename the icons.

https://git.gnome.org/browse/adwaita-icon-theme/tree/Adwaita/scalable/actions

> Are you happy for me to commit the modified patch on your behalf, or would
> you like to re-spin it yourself?

You can go ahead and commit; you don't have to wait for me.

> What's the etiquette here, do you know? Isn't it considered bad form for
> maintainers to mess around in translation team's files?

No, I'm pretty sure it's ok for a package maintainer to fix obvious bugs in translations. For instance, the former maintainer fixed a previous bug in the translation:

https://git.gnome.org/browse/geary/log/po/bs.po
Comment 5 Michael Gratton 2016-08-05 15:07:04 UTC
(In reply to Jeremy Bicha from comment #4)
> (In reply to Michael Gratton from comment #3)
> > Thanks for the patch! Turns out that the icons were already correctly named,
> > so under 3.20 simply removing the rtl tests seemed to work fine. I'd need to
> > check that was already the case under 3.14, though. There were also a couple
> > of missing commas that valac complained about, too.
> 
> That's interesting if the old icon names work, but the pattern used in
> adwaita-icon-theme has rtl as a (final) suffix so I think it would still
> make sense to rename the icons.

Yeah, actually, looks like I spoke too soon. I was looking at gnome-icon-theme-symbolic, which still uses "rtl-symbolic" somehow, and fumbled partly reverting the patch, which made it seem like it worked fine.

So anyway, your original patch does in indeed mostly work fine as long as the icons have been installed under $PREFIX. I had to build a .deb and install it to have them show up, but I'm pretty sure that is expected.

The indent icons on the composer's toolbar composer aren't being swapped however, but I think this is an Ubuntu/Ubuntu GNOME/Debian packaging bug: adwaita-icon-theme only ships adwaita-icon-theme.svg, and while adwaita-icon-theme-full contains RTL variants, they are named rather oddly: "format-indent-more-symbolic-rtl.symbolic.png". It's probably also worth just setting the indent action's icon names in composer.glade rather than in the source now, like the rest of the toolbar's actions, in any case.

I'll take a look at this further tomorrow, unless you beat me too it. :)

> > What's the etiquette here, do you know? Isn't it considered bad form for
> > maintainers to mess around in translation team's files?
> 
> No, I'm pretty sure it's ok for a package maintainer to fix obvious bugs in
> translations. For instance, the former maintainer fixed a previous bug in
> the translation:
> 
> https://git.gnome.org/browse/geary/log/po/bs.po

Oh, okay cool - I'll get that committed. Ta!
Comment 6 Jeremy Bicha 2016-08-05 19:29:32 UTC
I'm not sure I understand the issue you had with Ubuntu's (non-full) adwaita-icon-theme, but I'd sure like to know if it is missing essential icons. Please clarify what version of adwaita-icon-theme you are using and whether you are using Ubuntu's default themes with Unity.

Yes, the non-full adwaita-icon-theme includes the .svg icons but not the .png's. We (the Ubuntu desktop team) believe this saves several MB of space and we didn't think it caused issues.

format-indent-more-symbolic-rtl.symbolic.png is what upstream names it. For instance see https://apps.fedoraproject.org/packages/adwaita-icon-theme/contents
Comment 7 Michael Gratton 2016-08-06 01:43:11 UTC
Okay, after digging into this for ages, the problem turned out to be that my desktop was configured to use tango-icon-theme as its icon theme. It doesn't include any symbolic icons, so GTK presumably falls back to gnome-icon-theme-symbolic, which doesn't include an RTL version of the format-indent-* icons.

Switching to adwaita-icon-theme resolves the issue. Apologies for the noise, I'm still learning about this stuff.

Patches pushed to master as 509b669 and ac68a33. Thanks again!