Closed
Bug 1454323
Opened 6 years ago
Closed 6 years ago
Cmd+Q and other shortcuts are not working in some locales from Preferences
Categories
(Firefox :: Settings UI, defect, P1)
Tracking
()
VERIFIED
FIXED
Firefox 61
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox59 | --- | unaffected |
firefox60 | + | fixed |
firefox61 | --- | fixed |
People
(Reporter: flod, Assigned: zbraniecki)
Details
(Keywords: qawanted, regression, regressionwindow-wanted)
Attachments
(1 file)
59 bytes,
text/x-review-board-request
|
jaws
:
review+
jcristau
:
approval-mozilla-beta+
|
Details |
Realized while testing Japanese Nightly: if I open Preferences, focus one of the tabs and try to quit, I can't. The focus moves to the search field, and nothing happens. The same if I press other keyboard combinations, like cmd+T or cmd+N. At first I thought about an issue in the locale, but it happens also on Hebrew. - Nightly: ar, he, ja affected. Italian is NOT affected, which makes things more confusing. - Release for Japanese (59.0.2) is fine, so it's a regression
Reporter | ||
Comment 1•6 years ago
|
||
60 beta has the same issue. Marking as macOS only, since I have no way to test other platforms at the moment.
status-firefox59:
--- → unaffected
status-firefox60:
--- → affected
status-firefox61:
--- → affected
tracking-firefox60:
--- → ?
OS: Unspecified → Mac OS X
Reporter | ||
Comment 2•6 years ago
|
||
It gets weirder: I can reproduce easily with Nightly Japanese. But, if I open the Console before Preferences, then it starts working. And I see a bunch of warnings related to missing strings. Can it be caused by the log functions? It would also explain why Italian works (no missing strings). @zibi I have the feeling this is related to our work.
Flags: needinfo?(gandalf)
Reporter | ||
Comment 3•6 years ago
|
||
Some more details. Scenario #1: - Open Nightly - Open Preferences - Try to open a new tab with cmd+T Result: it doesn't work, search field is focused. If I open the console at this point, and close the main window, I get a weird "Navigated to chrome://extensions/content/dummy.xul". Also, the application menu is completely gone, there is only "Firefox Nightly". If I close the Console, menu comes back. If I open a new window, open the console, then Preferences, everything works as expected. Scenario #2. - Open Nightly - Open Console with cmd+shift+J - Open Preferences - Try to open a new tab with cmd+T Result: everything works as normal.
Reporter | ||
Comment 4•6 years ago
|
||
On second thoughts: 60 only has the General pane migrated to Fluent. All the rest of work, e.g. bug 1445084 on Search Results or bug 1420761, was done in 61, so it doesn't explain the issue showing up on Beta. French Nightly is affected too.
Summary: Cmd+Q and other shortcuts are not working in some locales → Cmd+Q and other shortcuts are not working in some locales from Preferences
Updated•6 years ago
|
Comment 5•6 years ago
|
||
This seems pretty bad. Tentatively marking this P1 for now.
Keywords: regressionwindow-wanted
Priority: -- → P1
Comment 6•6 years ago
|
||
flod, are you able to get a smaller regression window?
Flags: needinfo?(francesco.lodolo)
Reporter | ||
Comment 7•6 years ago
|
||
The first DevEdition (60b1) already has the problem, same for the fist official beta (60b3). I tried to pick a bunch of Nightlies, but it takes forever to download and extract from here. I don't think I can get smaller than this https://ftp.mozilla.org/pub/firefox/nightly/2018/02/ https://ftp.mozilla.org/pub/firefox/nightly/2018/03/ Working 60.0a1 (2018-02-27) (64 ビット) Built from https://hg.mozilla.org/mozilla-central/rev/505bafeafb66b0083ce1fca8eec2d061f1a5ebb7 Broken 60.0a1 (2018-03-01) (64 ビット) Built from https://hg.mozilla.org/mozilla-central/rev/0f8f71b0b9d84e7732c07f841e395de516b31b66 P.S. 2018-02-28-22-01-15-mozilla-central-l10n is broken (bug 1442145)
Flags: needinfo?(francesco.lodolo)
Reporter | ||
Comment 8•6 years ago
|
||
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=505bafeafb66b0083ce1fca8eec2d061f1a5ebb7&tochange=0f8f71b0b9d84e7732c07f841e395de516b31b66 A notable bug seems to be bug 1435912. But that's the bug that introduces Fluent strings in the General pane, so it might be just triggering an issue that it's already there. > But, if I open the Console before Preferences, then it starts working. And I > see a bunch of warnings related to missing strings. > > Can it be caused by the log functions? It would also explain why Italian > works (no missing strings).
Assignee | ||
Comment 9•6 years ago
|
||
I can't reproduce it on Nightly, mac when trying to use a `ja` language pack. Flod - can you confirm? STR: 1) Install Nightly 2) Install latest `ja` langpack for 61 from https://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla-central-l10n/win64/xpi/ 3) In Browser Console set `Services.locale.setRequestedLocales(["ja"]);` 4) Restart browser 5) Open Preferences 6) cmd+t Current Result: Opens new tab
Flags: needinfo?(gandalf) → needinfo?(francesco.lodolo)
Assignee | ||
Comment 10•6 years ago
|
||
Hmm, I can't also reproduce steps in scenario 1 from comment 3 on MacOS ja-JP-mac Nightly. Can someone else reproduce to reject the hypothesis that it's specific to Flod's setup?
Reporter | ||
Comment 11•6 years ago
|
||
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #9) > I can't reproduce it on Nightly, mac when trying to use a `ja` language > pack. Flod - can you confirm? Yes, I can replicate on a fresh profile with langpack installed (used intl.locale.requested in prefs since it was faster than remembering which key I need to change to get eval in Console)
Flags: needinfo?(francesco.lodolo)
Reporter | ||
Comment 12•6 years ago
|
||
For some reasons, it look harder to reproduce than this morning. Right now, I can get into this state more or less consistently by pressing cmd+, and then some keys while about:preferences is still loading.
Reporter | ||
Comment 13•6 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #12) > For some reasons, it look harder to reproduce than this morning. Right now, > I can get into this state more or less consistently by pressing cmd+, and > then some keys while about:preferences is still loading. It happens consistently on 60.0b12 (64 ビット) (latest Japanese beta), also on a new profile. Japanese currently has bug 1450656, so not sure if that emphasizes a race condition.
Assignee | ||
Comment 14•6 years ago
|
||
Yep, please, don't use ja-JP-mac beta 60 for this testing until bug 1450656 gets uplifted.
Assignee | ||
Comment 15•6 years ago
|
||
I'd like to ask someone to reproduce it to find if it's a unique setup of Flod's machine that causes it or mine that doesn't reproduce it. STR: 1) Download hebrew Firefox Nightly on Mac OS 2) Start on a fresh profile 3) Open Preferences 4) Press cmd+t Expected Result: New tab opens Current Result: Flod: Nothing happens Zibi: New tab opens
Keywords: qawanted
Reporter | ||
Comment 16•6 years ago
|
||
Small correction. 3) Open preferences via keyboard shortcuts (cmd+,), while the page is loading press other keys on the keyboard (I use cursor keys) Pressing any combination of keys with cmd (cmd+T, cmd+Q) focus the search field. If you close the windows using the mouse, you end up without menus on top, only "Firefox Nightly".
Comment 17•6 years ago
|
||
I get gandalf's result - with the 2018-04-16 Nightly he build on MacOS, I can't reproduce the primary bug.
Assignee | ||
Comment 18•6 years ago
|
||
Updated STR: 1) Download Hebrew Firefox Nightly on Mac OS https://download.mozilla.org/?product=firefox-nightly-latest-l10n-ssl&os=osx&lang=he 2) Start on a fresh profile 3) Press cmd+, to open Preferences 4) While Preferences are loading, press any key on the keyboard (e.g. cursor key) 5) After Preferences loaded, the search field is focused. 6) Press cmd+T Expected Result: New tab opens Current Result: Flod: Nothing happens Zibi: New tab opens I'm looking for someone to reproduce Flod's experience.
Comment 20•6 years ago
|
||
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #19) > Abe - can you help us verify this? Using the steps provided (in comment 18), I get a new tab. Test platform: OSX 10.13.4 MacBook Pro (Retina, 13-inch, Late 2013) Graphics: Intel Iris 1536 MB
Flags: needinfo?(amasresha)
Assignee | ||
Comment 21•6 years ago
|
||
Thank you Abe! Flod: what is the hardware/macos version you are using? Abe's and mine are both 2013 machines.
Status: NEW → UNCONFIRMED
Ever confirmed: false
Reporter | ||
Comment 22•6 years ago
|
||
I quite disagree that this bug is UNCONFIRMED, given that I can replicate on two completely different systems. One is a 2010 iMac with a mechanical disk and an after-marked SSD, the other is a 2016 MB Pro 13, so disk performance is not really comparable between the two. Model: iMac (27-inch, 2010) CPU: 2,8 GHz Intel Core i5 Memory: 12 GB GPU: ATI Radeon HD 5750 1024 MB Model: MacBook Pro (13-inch, 2016) CPU: 3,3 GHz Intel Core i7 Memory: 16 GB GPU: Intel Iris Graphics 550 1536 MB They both run 10.13.4 (17E199) in Italian. Are you both running macOS in English?
Assignee | ||
Comment 23•6 years ago
|
||
I used unconfirmed to mark that we are looking for others to confirm. The fact that you reproduce on two machines disqualifies some hypothesis but not others. Since both machines are yours it's much more likely that the root cause is unique to your setup than if we can find another person with their setup reproduce it and then find what's common between those setups while unique compared to mine, Abe's or Mike's.
Reporter | ||
Comment 24•6 years ago
|
||
(In reply to Francesco Lodolo [:flod] from comment #22) > They both run 10.13.4 (17E199) in Italian. Are you both running macOS in > English? Switched to English, including regional settings, rebooted and I still can reproduce. And just to make sure we're all on the same page. > 3) Press cmd+, to open Preferences > 4) While Preferences are loading, press any key on the keyboard (e.g. cursor key) #4 needs to happen while the page is still loading, which means you have to press cmd+, and cursor keys in one tight sequence. A few other things that I noticed: * The list of fonts takes a while to load the first time (the select remains empty while it's being populated, then switches to Arial). I only have 16 user fonts installed, so I don't see how that could be a problem. * When I press cmd+T, etc. the search field is focused, even if I see the menu bar flashing (like it was trying to call the right command).
Comment 25•6 years ago
|
||
I can reproduce this. It's way funky, and there are no traces to my eyes as to why that happens. I need a fresh start of the browser. Not necessarily a fresh profile. Then early enough, type something, and things get stuck. Only on that preference page, though. - Error console doesn't show anything - You can switch to different tabs, they work - Go back to pref tab, still broken - When focus is in the devtools bar on that page, keyboard shortcuts work One thing that just dawns on me, I press keys before the search box is shown, which means probably that XBL bindings aren't attached yet, but the DOM element is already there. Maybe?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 26•6 years ago
|
||
Also confirmed on http://archive.mozilla.org/pub/firefox/candidates/60.0b8-candidates/build1/mac/de/. Note, you need to be slow enough for the content to load, and quick enough for layout to not kick in.
Assignee | ||
Comment 27•6 years ago
|
||
Thanks Axel!
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
Assignee | ||
Comment 28•6 years ago
|
||
I was able to reproduce it on Windows in Nightly (pl), but not beta or release. It may be because on beta (pl) doesn't trigger any console warnings. I'll try to get a build with a patch that would delay console.warnings from Fluent until after localization is done. If it fixes things on nightly it should fix it on beta as well.
Assignee | ||
Comment 29•6 years ago
|
||
I spent some time testing various hypothesis around this today and I think I found out a reliable way to reproduce it, and to make it not happen, which, if confirmed will at least zero down the culprit. In order to reproduce this bug you need to launch a partially-localized build, so none of your regular en-US or it, but `pl` or `de` will work (at the moment, it can change as the localizers catch up). Then you need to open Preferences and mash buttons as it opens, and if you're doin it right, once the page loads you won't be able to press "cmd+t" to open new tab. Now, on to how to make it disappear: 1) Open the browser on which you reproduced the bug 2) Open browser console (cmd+j on mac) 3) Enter `Services.locale.setAvailableLocales(["pl"]);` 4) Then open Preferences and mash the buttons just the way you like 5) Press "cmd+t" and if I'm not mistaken a new tab is open flod: can you verify the steps to make it not reproduce?
Flags: needinfo?(francesco.lodolo)
Reporter | ||
Comment 30•6 years ago
|
||
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #29) > flod: can you verify the steps to make it not reproduce? I can confirm using Hebrew. * I can safely reproduce the bug (3/3 tries) if I open Preferences first. * Opening the console alone is not enough (correction to comment 2). * If I run Services.locale.setAvailableLocales(["he"]); in Console before opening Prefs, everything works normally (3/3 tries).
Flags: needinfo?(francesco.lodolo)
Assignee | ||
Comment 31•6 years ago
|
||
Great! I also tested if it's possible that just opening the console fixes it (which would indicate that initialization of console removes the issue) and tested if commenting out the console.warn we use fixes it. Neither does. That means, in my opinion that the race condition is present because what `setAvailableLocales` does is that it makes us not fallback on another locale. Here's how the process work in case of a complete localization: 1) Trigger opening Preferences 2) Preferences start loading and reach the Layout point 3) That triggers "MozBeforeInitialXULLayout" event which in turn triggers localization 4) C++ Node.localize collects localizable elements and triggers JS callback 5) JS callback collects l10n values and sends them back to C++ 6) C++ localizes the content 7) Loading continus 8) Happy ending Now, when we have incomplete locale, step (5) is more complicated: 5a) Collect l10n values for the main locale 5b) Notice missing values for some entries 5c) Trigger async I/O for the fallback locale 5d) Parse/format fallback locale and retrieve the remaining values 5e) Send combined values back to C++ This async I/O I suspect causes some shift in who wins the race condition around key bindings registration. I don't know why, but I strongly suspect that the race condition has always been there, it's just that we never had a fallback l10n so it was very hard or impossible to reproduce unless you were on a very, very slow hardware. Based on this, I suggest that: 1) Reasses the priority of the bug. It requires quite specific combination of factors including incomplete locale and pressing keys while Preferences are loading to trigger. While I want to fix it, I'm not sure if it deserves P1. 2) Involve DOM Team engineers to help us debug it. 3) If it's not fixable in XBL, consider remodelling Fluent DOM integration to submit the strings available in (5b) before moving on to (5c). But quite frankly, I'd prefer to fix it by finding out why pressing a key before preferences.xul <key> elements get localized messes with cmd+t/cmd+q bindings. Setting NI on :jaws to help me asses the severity and next steps.
Flags: needinfo?(jaws)
Comment 32•6 years ago
|
||
Is https://searchfox.org/mozilla-central/rev/59a9a86553e9bfd9277202748ff791fd9bc0713b/browser/components/preferences/in-content/preferences.xul#179 the only <key/> that is referenced by the preferences (not including subdialogs)? Would adding the <keyset/> and <key/> elements dynamically (after some point of loading) also fix this problem?
Flags: needinfo?(jaws)
Assignee | ||
Comment 33•6 years ago
|
||
Yep! That's brilliant, thank you Jared!
Comment hidden (mozreview-request) |
Assignee | ||
Comment 35•6 years ago
|
||
That fixes the Preferences UI, but we use the same logic in subdialogs, and I'd like to prevent having to risk someone somewhere not setting it since discovering it is hard. So I filed bug 1455379 to fix it in XUL. Yikes. I know.
Comment 36•6 years ago
|
||
mozreview-review |
Comment on attachment 8969355 [details] Bug 1454323 - Set the default key in <key> to avoid it capturing all command keys. https://reviewboard.mozilla.org/r/238096/#review243820 Great!
Attachment #8969355 -
Flags: review?(jaws) → review+
Comment 37•6 years ago
|
||
Pushed by zbraniecki@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/96867a07e1a3 Set the default key in <key> to avoid it capturing all command keys. r=jaws
Comment 38•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/96867a07e1a3
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Reporter | ||
Comment 39•6 years ago
|
||
I can't replicate anymore in 61.0a1 (2018-04-19). Setting NI for the uplift request.
Status: RESOLVED → VERIFIED
Flags: needinfo?(gandalf)
Assignee | ||
Comment 40•6 years ago
|
||
Comment on attachment 8969355 [details] Bug 1454323 - Set the default key in <key> to avoid it capturing all command keys. Approval Request Comment [Feature/Bug causing the regression]: bug 1424682 [User impact if declined]: it's possible for incomplete locale to lock command keys if keys are pressed during Preferences loading. [Is this code covered by automated tests?]: no [Has the fix been verified in Nightly?]: yes [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: Because it's a one line trivial change that has been eyeballed by enough experienced engineers and verified by the reporter to trust that we're not going to introduce any side regressions out of it. [String changes made/needed]: none
Flags: needinfo?(gandalf)
Attachment #8969355 -
Flags: approval-mozilla-beta?
Updated•6 years ago
|
Comment 41•6 years ago
|
||
Comment on attachment 8969355 [details] Bug 1454323 - Set the default key in <key> to avoid it capturing all command keys. fix a regression from using fluent in preferences, approved for 60.0b15
Attachment #8969355 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 42•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/49d46c15ca0b
You need to log in
before you can comment on or make changes to this bug.
Description
•