427729 - We should not mirror the Location Bar "Go" button for RTL

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Tomer Cohen :tomer]

Since we keep the location bar in LTR even when the rest of the GUI is RTL (The bookmarks star never jumps to the left in RTL) we should also keep the GO button (green arrow) pointing to the right, much like the English interface. 

Current en-US navigation bar: <back forward> () (X) [% http://example.com >] [G   ]

Current RTL navigation bar:   [  G] [% http://example.com <] (X) () <forward back>

Should be:  [  G] [% http://example.com >] (X) () <forward back>

Comment 1

[Tomer Cohen :tomer]

Attachment: [GTK and Win] removal of go-arrow-rtl.png (patch)

Please note it is also require to remove the image file go-arrow-rtl.png

Comment 2

[Dave Townsend [:mossop]]

Comment on attachment 315120 [details] [diff] [review]
[GTK and Win] removal of go-arrow-rtl.png (patch)

I am not an appropriate reviewer for this patch.

Additionally for the future a unified diff is the preferred form for patches.

Comment 3

[Behnam Esfahbod [:zwnj]]

Mano make a great RTL address-bar for fx2.  Why not implement it that way again?  A LTR Go button (a triangle points to right, read: back, previous, ...) in an RTL interface will be so annoying.

Comment 4

[Tomer Cohen :tomer]

(In reply to comment #3)
>  A LTR Go button (a triangle points to right, read: back, previous, ...)
> in an RTL interface will be so annoying.
> 

You right, but as Firefox 3 location bar contain some widgets, it will make it worse than the previous one, as the location bar is LTR. I don't think we should put the site icon in the middle of the bar, and the RSS feed icon will still be placed in the right side, so the most natural solution is to LTR the whole bar. 

I'm also not sure how many people will think the go button meant to be backward instead of go button, even as both are pointing to the same side.

Comment 5

[Axel Hecht [:Pike]]

Can we get a screenshot of what this fixes? Best before and after.

Comment 6

[Tomer Cohen :tomer]

Attachment: screenshot

(In reply to comment #5)
> Can we get a screenshot of what this fixes? Best before and after.
> 

Sure, no problem.

Comment 7

[Axel Hecht [:Pike]]

Comment on attachment 315814 [details]
screenshot

Mike, most of the awesomebar is LTR, thanks to URLs, the arrow, too?

Comment 8

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 315814 [details]
screenshot

I'm willing to take Tomer's lead, here. I don't think this blocks, but we'd take a patch.

Comment 9

[Tomer Cohen :tomer]

Please note that this bug is still open for RC1. Can we patch the theme before going final? 

Comment 10

[(no longer active)]

Comment on attachment 315120 [details] [diff] [review]
[GTK and Win] removal of go-arrow-rtl.png (patch)

I guess it's too late (unless we respin for RC2) but anyway asking Gavin for review.  Tomer: can you make a unified patch?

Comment 11

[Kai Liu]

Another possibility (along the lines of what Dao suggested for the placement of the dropdown button in Arabic vs. others) is for each of the RTL locales that want this to put it in their intl.css

Comment 12

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 315120 [details] [diff] [review]
[GTK and Win] removal of go-arrow-rtl.png (patch)

I don't have any opinion about whether this is better or worse than the current behavior. If we do this we should also stop packaging the icon (remove the three jar.mn lines that refer to it, and remove the file itself).

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 315120 [details] [diff] [review]
[GTK and Win] removal of go-arrow-rtl.png (patch)

a=beltzner, ceremonial last patch and demonstration of our dedication to non-en-US centrism

Comment 14

[Axel Hecht [:Pike]]

Checking in gnomestripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/gnomestripe/browser/browser.css,v  <--  browser.css
new revision: 1.218; previous revision: 1.217
done
Checking in winstripe/browser/browser.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/browser.css,v  <--  browser.css
new revision: 1.221; previous revision: 1.220
done

FIXED it is.

Comment 15

[(no longer active)]

(In reply to comment #13)
> (From update of attachment 315120 [details] [diff] [review])
> a=beltzner, ceremonial last patch and demonstration of our dedication to
> non-en-US centrism

Wow, it's really nice to see the (hopefully) last patch for Firefox 3 is to fix
an RTL issue...  :-)  Thanks Mike, Tomer and Axel!

Clearing the wanted1.9.0.x flag...

Comment 16

[Al Billings [:abillings - ex-MoCo]]

Verified with Mozilla/5.0 (X11; U; Linux i686; he; rv:1.9) Gecko/2008052912 Firefox/3.0 (RC2).