For information about this bug see bug 966395 and especially the comment at [1]. The issue for OSX has already been fixed. [1] - https://bugzilla.mozilla.org/show_bug.cgi?id=966395#c10

Moving old M2 P2 bugs to M4.

Move old M2's low-priority bugs to M6 milestone.

oops: old M2 P2 bugs should have been moved to new M4 milestone, not M6.

The duplicate bug 1191895 is a serious user-facing regression for RTL language users, including on OSX, in spite of comment 0 "the issue for OSX has already been fixed". For a while bug 1164693 concealed the problem, but since that was backed out enabling e10s now causes the directional marker on the caret to disappear. See bug 1216096 comment 11 onwards.

Jim, once the backout patches from bug 1216096 will land on aurora and beta, bidi caret marker will be broken in e10s. I am not sure where this is in terms of e10s scenario priority but to me it seems like a Beta blocker i.e. it should be fixed before 50% of beta population gets e10s enabled in 45 or later. Thoughts?

getting this into triage. sounds like an m8.

I set bidi.browser.ui to true, but I don't see any change with the caret. Simon, can you provide STR for this bug?

related, same bug on linux is bug 1033483.

(In reply to Jim Mathies [:jimm] from comment #8) > I set bidi.browser.ui to true, but I don't see any change with the caret. > Simon, can you provide STR for this bug? You also need an installed keyboard driver for a RTL language, e.g. Hebrew or Arabic.

My initial take on this is, this should be fixed when we ship e10s to the affected locales, eg Hebrew and Arabic. We should also avoid including affected locales in an e10s cohort, don't know if this is possible. needinfo'ing Pike - do you have a quick list of locales that need bidi in order to be usable?

"ar fa he ur" are the ones we have these days. Note, that list is useful for cohort-building, but I'd cringe if we hard-code it in a bunch of places.

(In reply to Axel Hecht [:Pike] from comment #12) > "ar fa he ur" are the ones we have these days. > > Note, that list is useful for cohort-building, but I'd cringe if we > hard-code it in a bunch of places. Cohort-building is (I think) what I'm interested in - if we can avoid these locales for the beta e10s cohorts then it affects the priority/milestone of the bug.

Having thought about this some more, my proposal is that this issue should block testing / deployment of e10s to these locales: ar, fa, he, ur I'm clearing my needinfo, and needinfo'ing Jim so this comes up in triage again and prioritized. I don't think this is a high priority for 46, but it should get fixed.

removing my ni so this falls into our triage list. lets find out how hard this will be to fix. We can file a bug on restricting e10s rollout based on locale if need be.

(In reply to Axel Hecht [:Pike] from comment #12) > "ar fa he ur" are the ones we have these days. > > Note, that list is useful for cohort-building, but I'd cringe if we > hard-code it in a bunch of places. Hey Axel, what is ur? or did you mean ru?

ur = Urdu https://en.wikipedia.org/wiki/Urdu

Ok thanks. Out of curiosity, is there a file somewhere that lists all the possible locales? I tried looking for it, and there are various files called "all-locales" on the tree, but they are all pretty incomplete. I wanted to know if any of these locales also have a "-AA" variant or if they always come with just these two letters

I'm not sure if there's a comprehensive list around, but the all-locales for DevEdition should cover most of them http://hg.mozilla.org/releases/mozilla-aurora/file/tip/browser/locales/all-locales Locale codes with language and region (ab-CD) are quite common, e.g. we have 4 Spanish regional variants. There are also some variants in Firefox OS world (e.g. sr-Latn, sr-Cyrl for Serbian, es for Spanish), but I don't think you're interested in those. This is a JSON of all locale code+description available in the Mozilla Localizations component https://l10n.mozilla-community.org/~flod/mozilla-l10n-query/?bugzilla=product

Review commit: https://reviewboard.mozilla.org/r/39441/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/39441/

Comment on attachment 8729402 [details] MozReview Request: Bug 1033488 - Send RTL information to child process by WM_INPUTLANGCHANGE. r?masayuki https://reviewboard.mozilla.org/r/39441/#review36125 I think that you should use nsBidiKeyboard::OnLayoutChange() from IMEInputHandler::OnCurrentTextInputSourceChange() too, though. # Wow, sorry for the delay due to fogetting to click "Publish Review"... Sigh.

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #21) > Comment on attachment 8729402 [details] > MozReview Request: Bug 1033488 - Send RTL information to child process by > WM_INPUTLANGCHANGE. r?masayuki > > https://reviewboard.mozilla.org/r/39441/#review36125 > > I think that you should use nsBidiKeyboard::OnLayoutChange() from > IMEInputHandler::OnCurrentTextInputSourceChange() too, though. > > # Wow, sorry for the delay due to fogetting to click "Publish Review"... > Sigh. Same issue on OSX is already fixed by bug 966395.

(In reply to Makoto Kato [:m_kato] from comment #22) > (In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #21) > > Comment on attachment 8729402 [details] > > MozReview Request: Bug 1033488 - Send RTL information to child process by > > WM_INPUTLANGCHANGE. r?masayuki > > > > https://reviewboard.mozilla.org/r/39441/#review36125 > > > > I think that you should use nsBidiKeyboard::OnLayoutChange() from > > IMEInputHandler::OnCurrentTextInputSourceChange() too, though. > > > > # Wow, sorry for the delay due to fogetting to click "Publish Review"... > > Sigh. > > Same issue on OSX is already fixed by bug 966395. I meant that you add a useful method in the patch, therefore, IMEInputHandler should use it too.

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #23) > (In reply to Makoto Kato [:m_kato] from comment #22) > > (In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #21) > > > Comment on attachment 8729402 [details] > > > MozReview Request: Bug 1033488 - Send RTL information to child process by > > > WM_INPUTLANGCHANGE. r?masayuki > > > > > > https://reviewboard.mozilla.org/r/39441/#review36125 > > > > > > I think that you should use nsBidiKeyboard::OnLayoutChange() from > > > IMEInputHandler::OnCurrentTextInputSourceChange() too, though. > > > > > > # Wow, sorry for the delay due to fogetting to click "Publish Review"... > > > Sigh. > > > > Same issue on OSX is already fixed by bug 966395. > > I meant that you add a useful method in the patch, therefore, > IMEInputHandler should use it too. This patch is for widget/windows. Do you mean that I should create widget/nsBaseBidiKeyboard class too or I should add same method to widget/cocoa/nsBidiKeyboard.mm?

Or I should add same method to WidgetUtils for all platforms?

(In reply to Makoto Kato [:m_kato] from comment #24) > (In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #23) > > (In reply to Makoto Kato [:m_kato] from comment #22) > > > (In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #21) > > > > Comment on attachment 8729402 [details] > > > > MozReview Request: Bug 1033488 - Send RTL information to child process by > > > > WM_INPUTLANGCHANGE. r?masayuki > > > > > > > > https://reviewboard.mozilla.org/r/39441/#review36125 > > > > > > > > I think that you should use nsBidiKeyboard::OnLayoutChange() from > > > > IMEInputHandler::OnCurrentTextInputSourceChange() too, though. > > > > > > > > # Wow, sorry for the delay due to fogetting to click "Publish Review"... > > > > Sigh. > > > > > > Same issue on OSX is already fixed by bug 966395. > > > > I meant that you add a useful method in the patch, therefore, > > IMEInputHandler should use it too. > > This patch is for widget/windows. Do you mean that I should create > widget/nsBaseBidiKeyboard class too or I should add same method to > widget/cocoa/nsBidiKeyboard.mm? Ah, sorry. I misunderstood. I understood as you adding OnLayoutChange() into XP level class. But not so actually. So, ignore my comments. Sorry for the confusion.

(In reply to Makoto Kato [:m_kato] from comment #25) > Or I should add same method to WidgetUtils for all platforms? It might be better. But it's okay not to do it here (there are more urgent bugs...)

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #27) > (In reply to Makoto Kato [:m_kato] from comment #25) > > Or I should add same method to WidgetUtils for all platforms? > > It might be better. But it's okay not to do it here (there are more urgent > bugs...) I will create the follow up bugs for Cocoa and Windows. Also it will be useful for bug 1033483.

When I switching en to ar, direction cursor does not change. Screenshot https://dl.dropboxusercontent.com/u/95157096/85f61cf7/t561exsbt3.png

(In reply to blinky from comment #31) > When I switching en to ar, direction cursor does not change. > > Screenshot > https://dl.dropboxusercontent.com/u/95157096/85f61cf7/t561exsbt3.png Are you using local build binary that you build from the latest source code of m-c or m-i?

Ah, bug 966395's fix is invalid too. Chrome process can send RTL information to content process correctly. But content process doesn't have bidi keyboard even if PuppetBidiKeyboard is implemented! I will fix it soon.

Actually, content process cannot get nsIBidiKeyboard by do_GetService("@mozilla.org/widget/bidikeyboard") on OSX and Windows. Since ContentChild will call PuppetBidiKeybaord directly, we should always register nsIBidiKeyboard by PuppetBidiKeyboard.

(In reply to Makoto Kato [:m_kato] from comment #32) > (In reply to blinky from comment #31) > > When I switching en to ar, direction cursor does not change. > > > > Screenshot > > https://dl.dropboxusercontent.com/u/95157096/85f61cf7/t561exsbt3.png > > Are you using local build binary that you build from the latest source code > of m-c or m-i? I am using the latest nightly m-c build.

Comment on attachment 8729402 [details] MozReview Request: Bug 1033488 - Send RTL information to child process by WM_INPUTLANGCHANGE. r?masayuki Approval Request Comment [Feature/regressing bug #]: Nothing [User impact if declined]: RTL caret doesn't show correctly on e10s mode with bidi.browser.ui=true. Due to this issue, we don't turn on e10s on RTL languages. [Describe test coverage new/current, TreeHerder]: Landed in m-c [Risks and why]: too low. This only works on bidi.browser.ui=true and e10s when keyboard layout is changed by user. [String/UUID change made/needed]: Nothing

Comment on attachment 8732052 [details] [diff] [review] Part 2. Make PuppetBidiKeyboard correctly on content process. Approval Request Comment [Feature/regressing bug #]: Nothing [User impact if declined]: RTL caret doesn't show correctly on e10s mode with bidi.browser.ui=true. Due to this issue, we don't turn on e10s on RTL languages and this is required for OSX too. [Describe test coverage new/current, TreeHerder]: Landed in m-c [Risks and why]: too low. This only works on bidi.browser.ui=true and e10s when keyboard layout is changed by user. [String/UUID change made/needed]: Nothing

Comment on attachment 8729402 [details] MozReview Request: Bug 1033488 - Send RTL information to child process by WM_INPUTLANGCHANGE. r?masayuki e10s + bidi, Aurora47+

https://hg.mozilla.org/releases/mozilla-aurora/rev/c8d83343c57b https://hg.mozilla.org/releases/mozilla-aurora/rev/7531defe6cb7

Hey Makoto, with this bug fixed can we turn e10s on for rtl users on windows?

(In reply to Jim Mathies [:jimm] from comment #42) > Hey Makoto, with this bug fixed can we turn e10s on for rtl users on windows? Yes. RTL caret and context menu for RTL work fine even if on e10s. But I don't know whether bug 1257731 is critical as Firefox team. But I think that this isn't critical because direction can change by context menu or another shortcut (accel+shift+x).

(In reply to Makoto Kato [:m_kato] from comment #43) > (In reply to Jim Mathies [:jimm] from comment #42) > > Hey Makoto, with this bug fixed can we turn e10s on for rtl users on windows? > > Yes. RTL caret and context menu for RTL work fine even if on e10s. > > But I don't know whether bug 1257731 is critical as Firefox team. But I > think that this isn't critical because direction can change by context menu > or another shortcut (accel+shift+x). Looks like you'll have that fixed in 50 so lets just target that release. I'll file a bug.