Closed
Bug 790486
Opened 12 years ago
Closed 12 years ago
Work - Alt + any other key is not detected from WinRT widget code
Categories
(Firefox for Metro Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: bbondy, Assigned: TimAbraldes)
References
Details
(Whiteboard: feature=work [completed-elm])
Attachments
(1 file)
(deleted),
patch
|
bbondy
:
review+
|
Details | Diff | Splinter Review |
Currently we aren't picking up any WinRT keypress combinations when alt is held down. This bug is to add platform support for key presses when alt is down.
Some keyboard shortcuts that will need this is alt+left and alt+right which should go back and forward in history.
Updated•12 years ago
|
Whiteboard: [metro-beta?]
Reporter | ||
Comment 1•12 years ago
|
||
I think the best route for this is as discussed at the meeting:
Provide alternate shortcuts not involving the alt key.
I'd like Jim to verify my claim that we can't easily add support for this with our WinRT browser first in case I missed something.
Updated•12 years ago
|
Product: Firefox → Firefox for Metro
Updated•12 years ago
|
Whiteboard: [metro-beta?] → [metro-mvp?]
Comment 2•12 years ago
|
||
ping
Reporter | ||
Comment 3•12 years ago
|
||
We aren't going to fight the platform for this one and instead provide alternate shortcut keys.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WONTFIX
Reporter | ||
Updated•12 years ago
|
Whiteboard: [metro-mvp?]
Reporter | ||
Comment 4•12 years ago
|
||
I noticed that IE10 Metro can use Alt + left and Alt+right to go forward and back so I think we need to look at this again.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Whiteboard: [metro-mvp][LOE:1]
Assignee | ||
Comment 5•12 years ago
|
||
Brian and I investigated handling "Alt" key presses at the Metro team meet-up this week. A patch is on the way.
Assignee: nobody → tabraldes
Assignee | ||
Comment 6•12 years ago
|
||
This patch fixes the issue on my machine with no obvious side effects. It changes MetroInput to handle the AcceleratorKeyActivated event instead of the KeyUp, KeyDown, and CharacterReceived events. AcceleratorKeyActivated is fired when any key is pressed, including alt (which is treated as a special "system key").
Attachment #703742 -
Flags: review?(netzen)
Assignee | ||
Comment 7•12 years ago
|
||
I should mention also that the patch hooks up alt+left and alt+right keyboard shortcuts but does not hook up any other keyboard shortcuts. I was thinking the other shortcuts could be hooked up in bug 785473, but I'd be willing to update this patch to hook up other alt keyboard shortcuts
Reporter | ||
Comment 8•12 years ago
|
||
Comment on attachment 703742 [details] [diff] [review]
Patch v1
Review of attachment 703742 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good!
::: widget/windows/winrt/FrameworkView.h
@@ +182,5 @@
> // rendering from that for print/preview in the different thread.
> //Microsoft::WRL::ComPtr<IWICImagingFactory2> mWicFactory;
> Microsoft::WRL::ComPtr<MetroApp> mMetroApp;
> Microsoft::WRL::ComPtr<ABI::Windows::UI::Core::ICoreWindow> mWindow;
> + Microsoft::WRL::ComPtr<ABI::Windows::UI::Core::ICoreDispatcher> mDispatcher;
nit: I think you can just do Microsoft::WRL::ComPtr<ICoreDispatcher> mDispatcher; here
::: widget/windows/winrt/MetroInput.cpp
@@ +219,5 @@
> + LogFunction();
> + Log(L"Accelerator key type: %d\n"
> + L"Accelerator key value: %d",
> + type,
> + vkey);
nit: Collapse this all to one line if you can with space.
@@ +1385,5 @@
> + coreAcceleratorKeys->add_AcceleratorKeyActivated(
> + WRL::Callback<AcceleratorKeyActivatedHandler>(
> + this,
> + &MetroInput::OnAcceleratorKeyActivated).Get(),
> + &mTokenAcceleratorKeyActivated);
nit: whitespace
Attachment #703742 -
Flags: review?(netzen) → review+
Updated•12 years ago
|
Blocks: 833155
Whiteboard: [metro-mvp][LOE:1] → [metro-mvp][LOE:1] feature=work
Assignee | ||
Comment 9•12 years ago
|
||
Landed on elm (with nits fixed):
https://hg.mozilla.org/projects/elm/rev/9e271ea22c71
First elm build containing the patch:
https://tbpl.mozilla.org/?tree=Elm&rev=9e271ea22c71
Whiteboard: [metro-mvp][LOE:1] feature=work → [metro-mvp][LOE:1][completed-elm] feature=work
Updated•12 years ago
|
Summary: Alt + any other key is not detected from WinRT widget code → Work - Alt + any other key is not detected from WinRT widget code
Whiteboard: [metro-mvp][LOE:1][completed-elm] feature=work → feature=work [completed-elm]
Comment 10•12 years ago
|
||
Resolving bugs in the Firefox for Metro product that are fixed on the elm branch. Sorry for the bugspam. Search your email for "bugspam-elm" if you want to find and delete all of these messages at once.
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
OS: Windows 8 Metro → Windows 8.1
You need to log in
before you can comment on or make changes to this bug.
Description
•