Context menu key events are generated with the wrong target

RESOLVED FIXED

Status

()

Core
Widget: Win32
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: neil@parkwaycc.co.uk, Assigned: neil@parkwaycc.co.uk)

Tracking

Trunk
x86_64
Windows Server 2008
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Assignee)

Description

8 years ago
When I try to open the context menu with the keyboard, it probably does not open, or if it does, it has the wrong context. This is because it fires with the document as its target. The popup manager then copies the incorrect target to document.popupNode. Instead the focused node should be the event target.

I tried in a build from a week ago and the event target when canvas has focus is actually the HTMLHtmlElement, so even then it's not the document.
(Assignee)

Comment 1

8 years ago
Turns out I was running an x86_64 build by mistake, which has a bug. Morphing.
Component: DOM: Events → Widget: Win32
Keywords: regression
OS: Windows XP → Windows Server 2008
QA Contact: events → win32
Hardware: x86 → x86_64
(Assignee)

Comment 2

8 years ago
Created attachment 461870 [details] [diff] [review]
Literal interpretation of MSDN
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #461870 - Flags: review?(jmathies)
(Assignee)

Comment 3

8 years ago
Created attachment 461872 [details] [diff] [review]
Possible patch

This "corrects" our cheat to correctly test all four bytes at once.
Attachment #461872 - Flags: review?(jmathies)
(Assignee)

Comment 4

8 years ago
Created attachment 461873 [details] [diff] [review]
Proposed patch

Since Windows actually passes -1, just test for -1...
Attachment #461873 - Flags: review?(jmathies)

Comment 5

8 years ago
I'm a fan of the literal MSDN interpretation, but proposed wfm as well.

Updated

8 years ago
Attachment #461870 - Flags: review?(jmathies) → review+

Updated

8 years ago
Attachment #461872 - Flags: review?(jmathies) → review-

Updated

8 years ago
Attachment #461873 - Flags: review?(jmathies) → review+
(Assignee)

Comment 6

8 years ago
Comment on attachment 461873 [details] [diff] [review]
Proposed patch

Seeking approval for a trivial Win64 correctness fix.
Attachment #461873 - Flags: approval2.0?
Comment on attachment 461873 [details] [diff] [review]
Proposed patch

Is there any way we can add a test to make sure that context menus brought up from the keyboard work?
(Assignee)

Comment 8

8 years ago
(In reply to comment #7)
> Is there any way we can add a test to make sure that context menus brought up
> from the keyboard work?
Not obviously, as this depends on being able to synthesize an OS event, the bug being that the OS event was incorrectly converted to a Gecko event.
Comment on attachment 461873 [details] [diff] [review]
Proposed patch

OK, that's too bad.
Attachment #461873 - Flags: approval2.0? → approval2.0+
(Assignee)

Comment 10

8 years ago
Pushed changeset c6c281075d0b to mozilla-central.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.