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 735890 - Fix the icons in RTL with new GTK+
Fix the icons in RTL with new GTK+
Status: RESOLVED FIXED
Product: gnome-music
Classification: Applications
Component: general
3.13.x
Other Linux
: Normal normal
: 3.14
Assigned To: gnome-music-maint
gnome-music-maint
Depends on:
Blocks:
 
 
Reported: 2014-09-02 14:07 UTC by Yosef Or Boczko
Modified: 2014-09-22 20:54 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Set required GTK+ version to 3.13.2 (773 bytes, patch)
2014-09-02 14:09 UTC, Yosef Or Boczko 		committed Details | Review
Fix the icons in RTL with last GTK+ (6.32 KB, patch)
2014-09-02 14:09 UTC, Yosef Or Boczko 		committed Details | Review
Custom handling for RTL icons in the notifi code (1.77 KB, patch)
2014-09-03 11:32 UTC, Yosef Or Boczko 		rejected Details | Review

Description Yosef Or Boczko 2014-09-02 14:07:47 UTC 
With the last GTK+ (from 3.13.2), GTK+ itself looking for the rtl variants of the icons.
It looking for name-icon-symbolic-rtl, with the suffix -rtl after -symbolic, not before.

Anyway, by the code in gnome-music, it looking for something like media-skip-backward-rtl-symbolic-rtl,
but it not exist, so we needs to remove the custom handler for RTL icons, and bump the GTK+ version.
Comment 1 Yosef Or Boczko 2014-09-02 14:09:38 UTC 
Created attachment 285150 [details] [review]
Set required GTK+ version to 3.13.2
Comment 2 Yosef Or Boczko 2014-09-02 14:09:52 UTC 
Created attachment 285151 [details] [review]
Fix the icons in RTL with last GTK+
Comment 3 Vadim Rutkovsky 2014-09-02 14:50:38 UTC 
Review of attachment 285150 [details] [review]:

LGTM
Comment 4 Vadim Rutkovsky 2014-09-02 14:52:11 UTC 
Review of attachment 285151 [details] [review]:

Some vars could be stripped and hardcoded, but since its important for upcoming release lets keep it like this
Comment 5 Yosef Or Boczko 2014-09-02 14:54:35 UTC 
Review of attachment 285150 [details] [review]:

Pushed as 181e628 - Set required GTK+ version to 3.13.2
Comment 6 Yosef Or Boczko 2014-09-02 14:54:52 UTC 
Review of attachment 285151 [details] [review]:

Pushed as 7cb4ec4 - Fix the icons in RTL with last GTK+
Comment 7 Yosef Or Boczko 2014-09-03 11:32:22 UTC 
Created attachment 285250 [details] [review]
Custom handling for RTL icons in the notifi code

gnome-shell not handle this yet, just GTK+ does.

It revert part of the previous patch.
Comment 8 Yosef Or Boczko 2014-09-03 11:39:14 UTC 
Review of attachment 285250 [details] [review]:

I miss there ':' character after 'else'.
Also, look like it never prepare the right icon, I see only text.
Comment 9 Yosef Or Boczko 2014-09-22 20:54:40 UTC 
There is a fix in gnome-shell for RTL icons too, so the notifi's icons look good again.