bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

"contextmenu" event has mozInputSource == MOZ_SOURCE_MOUSE even when hitting the context menu key on the keyboard

ASSIGNED
Assigned to

Status

()

Core
DOM: Events
P3
normal
ASSIGNED
2 years ago
10 months ago

People

(Reporter: squib, Assigned: squib)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(firefox53 affected)

Details

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
Per the bug title, the "contextmenu" event always claims to be from the mouse even when it's actually from the keyboard. That's because when the WidgetMouseEvent is created, the input source is never overridden: <https://dxr.mozilla.org/mozilla-central/rev/8103c612b79c2587ea4ca1b0a9f9f82db4b185b8/widget/MouseEvents.h#252>.

I have a test case and a patch that I'll upload shortly. This is my first (or maybe my second?) m-c bug, so I might get some of the review protocol wrong; if I do anything silly, just let me know!
(Assignee)

Comment 1

2 years ago
Created attachment 8819077 [details]
A simple test case

Here's a simple test case that logs the input source in the error console. Currently, it always outputs 1 (mouse), even when the event comes from the keyboard.
(Assignee)

Comment 2

2 years ago
Created attachment 8819081 [details] [diff] [review]
"contextmenu" event has mozInputSource == MOZ_SOURCE_MOUSE even when hitting the context menu key on the keyboard
(Assignee)

Comment 3

2 years ago
Comment on attachment 8819081 [details] [diff] [review]
"contextmenu" event has mozInputSource == MOZ_SOURCE_MOUSE even when hitting the context menu key on the keyboard

Hopefully I flagged the right person for review and managed to upload this patch correctly. Where should I put tests for this? I'm not super-familiar with how m-c organizes its tests.
Attachment #8819081 - Flags: feedback?(bugs)
Comment on attachment 8819081 [details] [diff] [review]
"contextmenu" event has mozInputSource == MOZ_SOURCE_MOUSE even when hitting the context menu key on the keyboard

It is not super important where to put tests. Possibly dom/events/test/*

You aren't setting button to any value when mContextMenuTrigger == eContextMenuKey.
Attachment #8819081 - Flags: feedback?(bugs) → feedback+
(Assignee)

Comment 5

2 years ago
(In reply to Olli Pettay [:smaug] from comment #4)
> It is not super important where to put tests. Possibly dom/events/test/*

Thanks! I'll look there and see if I can cargo-cult an existing test into something that checks this.

> You aren't setting button to any value when mContextMenuTrigger ==
> eContextMenuKey.

I was debating what to do here; It's automatically set to 0 in the WidgetMouseEventBase constructor (as is inputSource)[1], but it's theoretically possible that that would change in the future and then we'd have an uninitialized member. I suppose I'd want to either a) do what I'm doing now and rely on the base class to initialize its members, or b) set `button` *and* `inputSource` in both branches. I'm open to either; is there a general guideline I should follow here for Mozilla code?

[1] https://dxr.mozilla.org/mozilla-central/rev/8103c612b79c2587ea4ca1b0a9f9f82db4b185b8/widget/MouseEvents.h#94
The general guideline is something along the lines "whatever makes the code easiest to read and maintain, unless there are some performance or security related reasons to do otherwise".
So, in this case explicit initialization would be nice.

Updated

10 months ago
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.