979653 - Ctrl+Shift+X does not work in URL bar in RTL locales

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect)

Description

[Yaron]

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:29.0) Gecko/20100101 Firefox/29.0 (Beta/Release)
Build ID: 20140303004002

Steps to reproduce:

Ctrl+Shift+X does not work in URL bar.

I use RTL (Hebrew), and it's important for me to be able to change direction.

Comment 1

[moses]

Hi,

This bug is too vague. Can you provide more information?
When does it happen?
Does it happen in a certain Firefox version?

Comment 2

[Yaron]

FF 29.
The text direction is FIXED to the right.  

In previous versions (FF 27, I haven't tried v28) it was possible to change text direction in the URL bar.

Comment 3

[moses]

Can you go to "about:config" and search for "bidi.browser.ui" and set that from "false" to "true"

restart Firefox then try CTRL + SHIFT + X

Comment 4

[moses]

Here's another tip

Go to "about:config" type "bidi.direction" set that from "1" to "2" and restart.

Comment 5

[Yaron]

Thank you.

It doesn't work.

Comment 6

[Yaron]

Hi Moses,

Could you please update me?
This issue is highly important to me.

Thanks.

Comment 7

[:Gijs (he/him)]

This works on 28, it seems...

Comment 8

[:Gijs (he/him)]

Uh, interestingly, it seems the shortcut still works when using an LTR build, but not when using an RTL build... :-\

Comment 9

[:Gijs (he/him)]

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9bcc52594322&tochange=298f262f21ff

Comment 10

[:Gijs (he/him)]

This looks like bug 675579. Ehsan, can you have a look, please? The search bar works, but the URL bar doesn't, and this is quite a bad regression. The issue only appears in RTL builds (or in LTR builds with the "Force RTL" add-on installed and the build set to behave as RTL). We'll need to move fast in order to still fix this for 29. :-(

Comment 11

[:Gijs (he/him)]

Final regression pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9bcc52594322&tochange=7eb615c086e8

Comment 12

[(no longer active)]

Yes, I'd be happy to fix this.  Can you please point me to the code which deals with the location bar direction?  Bug 675579 might be breaking the assumptions of that code...

Comment 13

[:Gijs (he/him)]

(In reply to :Ehsan Akhgari (lagging on bugmail, needinfo? me!) from comment #12)
> Yes, I'd be happy to fix this.  Can you please point me to the code which
> deals with the location bar direction?  Bug 675579 might be breaking the
> assumptions of that code...

I am not aware of there being specific code for it... it will just use the generic cmd_switchTextDirection editor command, AFAICT.

Comment 14

[:Gijs (he/him)]

Huh, actually, it seems like:

1) we set the dir=ltr/rtl attribute correctly in the editor switch text direction code;
2) the CSS in this media query:

http://mxr.mozilla.org/mozilla-central/source/layout/style/forms.css#82

is broken because the dir attribute gets set on the input element instead of the anonymous div now (per attachment 8360729 [details] [diff] [review] from bug 675579.

Ehsan, is that right?

Comment 15

[:Gijs (he/him)]

Attachment: fix dir attribute checks for url field in rtl mode,

Might as well make this a patch. This addresses the regression. I did notice that, in RTL mode, it seems the first ctrl-shift-x call has no effect, presumably because we end up setting the dir attribute to rtl instead of ltr, even though the document is in rtl mode already and so this doesn't actually switch text direction. However, that was an issue before and I don't think we should block this bug on it. I'm happy to file a followup if that makes sense (I'm also not sure how we'd fix it - presumably we'd need to look at computed style, which would trigger layout flushes, which makes me sad)

Comment 16

[(no longer active)]

(In reply to :Gijs Kruitbosch from comment #15)
> Might as well make this a patch. This addresses the regression. I did notice
> that, in RTL mode, it seems the first ctrl-shift-x call has no effect,
> presumably because we end up setting the dir attribute to rtl instead of
> ltr, even though the document is in rtl mode already and so this doesn't
> actually switch text direction. However, that was an issue before and I
> don't think we should block this bug on it. I'm happy to file a followup if
> that makes sense (I'm also not sure how we'd fix it - presumably we'd need
> to look at computed style, which would trigger layout flushes, which makes
> me sad)

Please file a follow-up on it.  I actually thought that the editor code takes the direction of the element into account properly, there might be a bug in that logic.

Comment 17

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/bf99640d19e5

Comment 18

[Yaron]

In FF versions prior to v29:

The first Ctrl+Shift+X call indeed has no effect.
I've been using a small extension calling cmd_switchTextDirection TWICE on each new window.
Aligning the URL bar text to the right in RTL mode makes no sense; - we hardly ever use non-Latin characters in that bar.

*

If you are going to fix the regression in v29, it might be a good idea to change the default alignment to the left.

Thanks.

Comment 19

[Yaron]

This issue is frequently discussed in RTL forums.
Many users often ask to align the text to the left.

Comment 20

[(no longer active)]

Yaron, please file a new bug for the directionality issue and CC me on it.  Thanks!

Comment 21

[Yaron]

Hi Ehsan,

https://bugzilla.mozilla.org/show_bug.cgi?id=993806

Just to save me some time, where is forms.css located in omni.ja?

Thanks.

Comment 22

[(no longer active)]

(In reply to Yaron from comment #21)
> Hi Ehsan,
> 
> https://bugzilla.mozilla.org/show_bug.cgi?id=993806

Thanks!

> Just to save me some time, where is forms.css located in omni.ja?

On my OS X machine, it's at ./chrome/toolkit/res/forms.css.  I think the location should be the same in all platforms.

Comment 23

[Yaron]

Thank you. I appreciate it.
And thanks for fixing the regression.

Comment 24

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bf99640d19e5

Comment 25

[Yaron]

Thank you Gijs and Ehsan.

Comment 26

[:Gijs (he/him)]

Comment on attachment 8403581 [details] [diff] [review]
fix dir attribute checks for url field in rtl mode,

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 675579
User impact if declined: users of RTL locales in Firefox are stuck with a right-aligned location bar
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): low, tiny CSS change to reflect changes made in bug 675579
String or IDL/UUID changes made by this patch: none

Comment 27

[Marco Mucci [:MarcoM]]

Hi Anthony, can you review if this resolved bug will need further verification.  Thanks.

Comment 28

[u279076]

We should definitely spotcheck this with a few RTL locales. Florin, please make sure this gets assigned and tested.

Comment 29

[Catalin Varga [QA][:VarCat]]

Can I please get some RTL locales in order to verify the bug (at least Hebrew and Arabic).
This bug is fixed on Nightly so on ftp no locales are available.

Comment 30

[u279076]

(In reply to Catalin Varga [QA][:VarCat] from comment #29)
> Can I please get some RTL locales in order to verify the bug (at least
> Hebrew and Arabic).
> This bug is fixed on Nightly so on ftp no locales are available.

Arabic and Hebrew should be a good enough sample size. You can get l10n builds from ftp://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla-central-l10n/

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ef2f3afc30af
https://hg.mozilla.org/releases/mozilla-beta/rev/44a94313968a

Comment 32

[Catalin Varga [QA][:VarCat]]

I've verified this bug using:
FF 31 ar and he localization
Build id: 20140410030200
OS: Win 7 x64, Mac Os x 10.9, Ubuntu 13.04 x64

I will verify for Aurora and Beta when localized builds containing the fix will be available for all operating systems.

Comment 33

[cvarga]

I've verified this bug using:

FF 30 aurora ar and he localization
Build id: 20140411004002
OS: Win 7 x64, Mac Os x 10.9, Ubuntu 13.04 x64

FF 29.0b7 ar and he localization
Build id: 20140410150427
OS: Win Xp x86, Mac Os x 10.9, Ubuntu 13.04 x64

Comment 34

[Yaron]

FF 32 - The hook-indicator is only half displayed (1px).  
Why not change the code and show 2px?