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 700543 - Broken RTL handling for some icons
Broken RTL handling for some icons
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: general
3.8.x
Other Linux
: Normal normal
: ---
Assigned To: Control-Center Maintainers
Control-Center Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-05-17 17:43 UTC by Bastien Nocera
Modified: 2013-06-05 09:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
shell: Set previous button arrow according to locale's text direction (1.21 KB, patch)
2013-05-20 13:30 UTC, Rui Matos
needs-work Details | Review
Set previous buttons arrow icon according to locale's text direction (1.71 KB, patch)
2013-05-21 10:10 UTC, Rui Matos
none Details | Review
Set button arrow icons according to locale's text direction (4.06 KB, patch)
2013-05-21 13:35 UTC, Rui Matos
committed Details | Review

Description Bastien Nocera 2013-05-17 17:43:37 UTC
$ git grep go-previous-symbolic
panels/region/cc-input-chooser.c:#define ARROW_PREV "go-previous-symbolic"
panels/wacom/cc-wacom-nav-button.c:     image = gtk_image_new_from_icon_name ("go-previous-symbolic", GTK_ICON_SIZE_MENU);
shell/cc-window.c:                                           "go-previous-symbolic");
Comment 1 Rui Matos 2013-05-20 13:30:43 UTC
Created attachment 244771 [details] [review]
shell: Set previous button arrow according to locale's text direction

--

(In reply to comment #0)
> $ git grep go-previous-symbolic
> panels/region/cc-input-chooser.c:#define ARROW_PREV "go-previous-symbolic"

That one is fine already.

> shell/cc-window.c:
> "go-previous-symbolic");

Here's a patch for this.
Comment 2 Bastien Nocera 2013-05-21 06:18:22 UTC
Review of attachment 244771 [details] [review]:

::: shell/cc-window.c
@@ +1409,3 @@
   priv->previous_button = button = gd_header_simple_button_new ();
   gd_header_button_set_symbolic_icon_name (GD_HEADER_BUTTON (button),
+                                           rtl ? "go-next-symbolic" : "go-previous-symbolic");

The correct icon name is "go-next-rtl-symbolic", not "go-previous-symbolic".
Comment 3 Bastien Nocera 2013-05-21 06:19:24 UTC
(In reply to comment #1)
> Created an attachment (id=244771) [details] [review]
> shell: Set previous button arrow according to locale's text direction
> 
> --
> 
> (In reply to comment #0)
> > $ git grep go-previous-symbolic
> > panels/region/cc-input-chooser.c:#define ARROW_PREV "go-previous-symbolic"
> 
> That one is fine already.

I'd rather it was fixed to use go-next-rtl rather than go-previous.
Comment 4 Rui Matos 2013-05-21 10:10:31 UTC
Created attachment 244904 [details] [review]
Set previous buttons arrow icon according to locale's text direction

--

> The correct icon name is "go-next-rtl-symbolic", not "go-previous-symbolic".

Interesting, I didn't know about these -rtl- variants. The patch now
fixes both the occurences that I can test here.
Comment 5 Rui Matos 2013-05-21 13:35:58 UTC
Created attachment 244921 [details] [review]
Set button arrow icons according to locale's text direction

--

Ugh, this stuff can be confusing. Ok, now this fixes all the
occurrences. Tested the wacom panel with test-wacom.
Comment 6 Joaquim Rocha 2013-05-21 14:08:45 UTC
Thanks for also doing it for Wacom, Rui.
Comment 7 Bastien Nocera 2013-06-04 17:20:54 UTC
Review of attachment 244921 [details] [review]:

All three look good. Could you please commit them separately though?
Comment 8 Rui Matos 2013-06-05 09:27:12 UTC
Pushed as 3 separate commits.