631978 - Forward and back arrows reversed in RTL UI

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Simon Montagu :smontagu]

Attachment: Screenshot

In RTL UI on Linux, the forward and back arrows are reversed -- the arrows are in the correct positions, but the back arrow points to the left and the forward arrow to the right.

Regression range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange094a7967e171&tochange=847a825087f2, presumably bug 621962. I am using the Oxygen icon theme under Ubuntu 10.10, but I got the same results in a new user with nothing customized (except intl.uidirection.en set to rtl in Firefox), i.e. with Ambience desktop theme and Ubuntu-Mono-Dark icons.

Not sure if it makes any difference, but my desktop as a whole is LTR, and only Firefox is using RTL UI.

Comment 1

[Karl Tomlinson (:karlt)]

Thanks Simon.  gnome-icon-theme even has the same issue.

GTK now looks up only the new "standard" icon names (and provides only icons with the new not-quite "standard" names), but most themes (perhaps all except GTK's default hicolor theme) provide only a single icon for the "standard" name because the spec doesn't state how to provide bidi variations.  GTK seems to be trying to set a new "standard" but the icon themes have not joined the revolution.

https://bugzilla.gnome.org/show_bug.cgi?id=629878#c19

Comment 2

[Karl Tomlinson (:karlt)]

I was thinking about reflecting icons as is done on Mac and NT, but Owen Taylor is not so keen on that.
http://lists.freedesktop.org/archives/xdg/2004-October/003479.html

There was a similar convention proposed for rtl icon names in 2004, but it doesn't seem to have made it into the spec.
http://lists.freedesktop.org/archives/xdg/2004-October/003482.html

Comment 3

[Karl Tomlinson (:karlt)]

dbusmenu seems to be trying to use the same new convention as GTK.
http://bazaar.launchpad.net/~dbusmenu-team/dbusmenu/trunk/revision/51

I wonder which themes actually provide such icons.

Comment 4

[Amiad]

It's bug in gnome 2.20.
https://bugzilla.gnome.org/show_bug.cgi?id=632775
https://bugs.launchpad.net/ubuntu/+source/nautilus/+bug/664194

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Not blocking given it's a GTK bug and occurs in key GTK apps such as Nautilus.

Maybe we can ask a GTK person if they can recommend a workaround here? If there's a clean way to flip as a workaround, and the GTK folks are OK with it, that might make sense.

Comment 6

[Simon Montagu :smontagu]

Bug compatibility with Nautilus here is still a regression in Firefox. See https://bugzilla.gnome.org/show_bug.cgi?id=632775#c1 where the reporter attaches a screenshot of Firefox without the bug.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I understand that, but if we're going to integrate with platform functionality then sometimes we're going to be affected by bugs in that functionality. Sometimes that's going to have to trump our goal of not regressing; we can't work around everything :-(.

Although I'd *like* to have better RTL UI support on GTK than GTK itself does, I don't think we should block on it.

(Often we choose to bypass platform functionality precisely so we don't get stuck in this situation, but here we don't really have that option.)

Comment 8

[(no longer active)]

(In reply to comment #7)
> Although I'd *like* to have better RTL UI support on GTK than GTK itself does,
> I don't think we should block on it.

Even considering the fact that this has regressed in Firefox 4, and Firefox 3.6 would look fine?

That's probably one of the biggest ways that our RTL interface could seem broken on Linux.  I'd say that we should maybe take a theme patch which just reverses the image without using the "-ltr" variant... :/

Comment 9

[(no longer active)]

I thought about this a bit more, and I think we should block on this.  We shouldn't be blocking on getting gtk+ fixed, but as far as our users are concerned, this is something which got broken in Firefox 4, and it's visible in our primary interface.  I'd say that this matches the criteria for blocking on something (technical reasons aside), doesn't it?

Comment 10

[Karl Tomlinson (:karlt)]

Attachment: check old icon names first for bidi icons

This first looks up bidi icons by the old icon names; if they are not found,
we fall back to using the stock id, which would use the new icon names.

This wouldn't do exactly what we want if there were a new theme that only
implemented the new icon names but inherited from a theme that implemented old
icon names.  That's probably most likely to happen if a distribution patches GTK's default theme for backward compatibility, so I've handled that case.  Other situations seem unlikely and consequence not too bad.

What we really wanted to do is look up by both old and new icon names,
preferring the old names if they exist at the same level of inheritance as a
new icon name.  gtk_icon_theme_choose_icon does this search for us, but I
don't know how to query GTK for the new icon names.  We'd need to get a
GtkIconSource from the GtkIconSet, but there doesn't seem to be any means
provided to do that.

With this patch we no longer do exactly what GTK does for it's bidi stock
icons, but we do what icon themes are expecting GTK applications to do.

In bug 621962 part 4, I removed the saving of GtkIconSets thinking that path
was not taken (and because it was difficult to keep that behavior).  I haven't
added that back in, so we loose the caching of the (unpremultiplied)
GdkPixbufs when the style has applied an effect to the icon to indicate
insensitive state.  It looks like the pre-styled icon is still cached, so it's
only the processing (rather than file lookup) that must be repeated.  That
doesn't seem such a bad thing, given it saves a little in memory.

Comment 11

[Karl Tomlinson (:karlt)]

The patch applies on top of attachment 510470 [details] [diff] [review].

Comment 12

[Karl Tomlinson (:karlt)]

https://bugzilla.gnome.org/show_bug.cgi?id=632775
was just closed with this commit:
http://git.gnome.org/browse/gtk+/commit/?id=3a50b460c6c64377d7402248e55b192d5c27ae2a

Not yet clear on how that works or whether it will be fixed on branch, but that change deals with GTK+-3-specific code.

Comment 13

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/46b44e59d2c5

Comment 14

[Amiad]

The bug happen in gtk+ 3 again.
https://bugzilla.gnome.org/show_bug.cgi?id=632775
https://bugs.launchpad.net/ubuntu/+source/totem/+bug/1048273

screenshot: http://i.imgur.com/p26l9.png

Comment 15

[Tomer Cohen :tomer]

(In reply to Amiad from comment #14)
> The bug happen in gtk+ 3 again.

Does it affect current Mozilla Firefox binaries or distribution-packed Firefox? If not, let's keep the noise to a new bug which will block bug 627699. Elad is following the GTK3 porting bug, so I guess he'd notify us if (and when) something breaks with the RTL UI on GTK3.