Closed Bug 1297993 Opened 5 years ago Closed 5 years ago

Warnings for "key events not available" should include the key ID

Categories

(Core :: XBL, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: jaws, Assigned: jaws)

References

(Depends on 1 open bug)

Details

Attachments

(1 file)

Steps to reproduce:
On Windows 10 Nightly 2016-08-23 51.0a1
Open Nightly
Open the Browser Console
See the following warnings:
Key event not available on some keyboard layouts: key=“c” modifiers=“accel,alt”
Key event not available on some keyboard layouts: key=“r” modifiers=“accel,alt”
Key event not available on some keyboard layouts: key=“i” modifiers=“accel,alt,shift”
The first warning was just removed by https://hg.mozilla.org/mozilla-central/rev/0860fa85235a so that doesn't apply anymore.

The second warning was introduced by https://hg.mozilla.org/mozilla-central/rev/c2849a432eee because on Windows this translates to Ctrl+Shift+R which is also used for Browser:ReloadSkipCache.
Blocks: 1144749
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #1)
> The second warning was introduced by
> https://hg.mozilla.org/mozilla-central/rev/c2849a432eee because on Windows
> this translates to Ctrl+Shift+R which is also used for
> Browser:ReloadSkipCache.

I'm confused. Accel-alt-r is ctrl-alt-r - why would that translate to ctrl-shift-r ?
Flags: needinfo?(jaws)
The third warning comes from https://hg.mozilla.org/integration/fx-team/rev/9b9b8514c6b0479d470b0c30f84b6e9552591da5

I've updated nsXBlPrototypeHandler.cpp to include the key ID to help with tracking down these regressions.

(In reply to :Gijs Kruitbosch (away 26-29 incl.) from comment #2)
> (In reply to Jared Wein [:jaws] (please needinfo? me) from comment #1)
> > The second warning was introduced by
> > https://hg.mozilla.org/mozilla-central/rev/c2849a432eee because on Windows
> > this translates to Ctrl+Shift+R which is also used for
> > Browser:ReloadSkipCache.
> 
> I'm confused. Accel-alt-r is ctrl-alt-r - why would that translate to
> ctrl-shift-r ?

Yes, you are right. I don't see an actual key conflict here in my usage. The key conflict detection code is very primitive, it will warn about anything with only ctrl and/or alt as the keyboard modifiers on Windows.
Blocks: 1248603
Flags: needinfo?(jaws)
On Windows, there is no AltGr modifier state. Instead, Windows represents its state with Ctrl+Alt. Therefore, nsXBLPrototypeHandler warns such shortcut key handlers because if the combination causes text input, both Ctrl and Alt modifier state are not set to true by widget. Therefore, with some keyboard layouts, Ctrl+Alt+[A-Z] may not be available.

# Sorry for the delay to review due to my business trip and having been sick yesterday.
Comment on attachment 8784799 [details]
Bug 1297993 - Include the ID of the offending <key> element in the key conflict message.

https://reviewboard.mozilla.org/r/74110/#review73412

::: dom/locales/en-US/chrome/layout/xbl.properties:6
(Diff revision 1)
>  # LOCALIZATION NOTE: do not localize key=“%S” modifiers=“%S”
> -GTK2Conflict=Key event not available on GTK2: key=“%S” modifiers=“%S”
> -WinConflict=Key event not available on some keyboard layouts: key=“%S” modifiers=“%S”
> +GTK2Conflict2=Key event not available on GTK2: key=“%S” modifiers=“%S” id=“%S”
> +WinConflict2=Key event not available on some keyboard layouts: key=“%S” modifiers=“%S” id=“%S”

Do you really need to change the key names of these resource? (If you did it for easier to review, I'm really happy, though.) IIRC, l10n tools detect which keys' values are changed even without changing the key names...

And please add id=%S to the "LOCALIZATION NOTE:" too.
Attachment #8784799 - Flags: review?(masayuki) → review+
Thanks for the review and for catching the localization note change. I changed the keys because https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Updating_Entity_Names says that l10n tools do not detect key value changes and that they only look at key names.
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Whiteboard: [keep-open]
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/286a80b93223
Include the ID of the offending <key> element in the key conflict message. r=masayuki
This varies by platform. I'm not sure what more can be done but at least the patch that landed helps make debugging this easier. I'm going to close this bug now and file a new bug since this patch has landed quite some time ago.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Summary: Warnings for key events not available appear on startup on Windows 10 → Warnings for "key events not available" should include the key ID
Whiteboard: [keep-open]
Component: Untriaged → XBL
Product: Firefox → Core
Target Milestone: --- → mozilla51
Filed bug 1303876 for the follow-up.
Depends on: 1303876
You need to log in before you can comment on or make changes to this bug.