Unable to specify XBL modifiers for all possible mouse events

RESOLVED FIXED

Status

()

Core
XBL
RESOLVED FIXED
6 years ago
6 years ago

People

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

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
XBL special-cases key and mouse events so that it can check for modifiers. Key events are special-cased in nsXBLBinding::InstallEventHandlers, while mouse events are special-cased in NS_NewXBLEventHandler. However I noticed that DOMMouseScroll is missing from this list, which (for instance) means that instead of being able to use modifiers="accel", search.xml needs to #ifdef its modifier based on the platform.

I don't have an exhaustive list of mouse event names so I don't know whether any other mouse events are missing, thus CCing some people who might know.
(Assignee)

Comment 1

6 years ago
Created attachment 517279 [details] [diff] [review]
Patch for DOMMouseScroll only
At least also: MozMouseHittest, dragstart, drag, dragleave, drop, dragend, popupshowing, popupshown, popuphiding, popuphidden

Plus, all of the gesture and touch events listed at
  http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsContentUtils.cpp#675

It may be easier and more efficient just to add an indicator the events that are or inherit from mouse event to the table in nsContentUtils::InitializeEventTable.
(Assignee)

Comment 3

6 years ago
(In reply to comment #2)
Thanks for the list!

> At least also: MozMouseHittest
Surely it makes no sense to consider modifiers in this case?

> dragstart, drag, dragleave, drop, dragend
Ah, those are the new drag events? I see the old ones are there already.

> popupshowing, popupshown, popuphiding, popuphidden
I'm not convinced that we need to consider modifiers. IIRC these are only mouse events because they need to have coordinates.

> Plus, all of the gesture and touch events
Again, I remain unconvinced.

> It may be easier and more efficient just to add an indicator the events that
> are or inherit from mouse event to the table in
> nsContentUtils::InitializeEventTable.
Unfortunately that table has "on" prefixes for all of its event atom names.
The list is just the events I saw that inherit from mouse event. I don't even know what some of the events are for.

The drag events at least should be added.

Comment 5

6 years ago
(In reply to comment #3)
> > It may be easier and more efficient just to add an indicator the events that
> > are or inherit from mouse event to the table in
> > nsContentUtils::InitializeEventTable.
> Unfortunately that table has "on" prefixes for all of its event atom names.

You can access entries in the table either with onfoo atom, or using foo string.
see sAtomEventTable and sStringEventTable
(Assignee)

Comment 6

6 years ago
Created attachment 517404 [details] [diff] [review]
Patch for mouse, scroll and drag events
Attachment #517404 - Flags: feedback?(enndeakin)
Attachment #517404 - Flags: feedback?(Olli.Pettay)

Comment 7

6 years ago
Comment on attachment 517404 [details] [diff] [review]
Patch for mouse, scroll and drag events


>   /**
>+   * Return the category for the event with the given name. The name is the
>+   * event name *without* the 'on' prefix. Returns NS_EVENT if the event
>+   * doesn't match a known event name.
>+   *
>+   * @param aName the event name to look up
>+   */
>+  static PRUint32 GetEventType(const nsAString& aName);

Could you call this GetEventStructType? Or
GetEventCategory()


>+  switch (nsContentUtils::GetEventType(nsDependentAtomString(aEventType))) {
>+    case NS_MOUSE_EVENT:
>+    case NS_MOUSE_SCROLL_EVENT:
>+    case NS_DRAG_EVENT:
Add also NS_SIMPLE_GESTURE_EVENT


Some tests, please.
Attachment #517404 - Flags: feedback?(Olli.Pettay) → feedback+
Comment on attachment 517404 [details] [diff] [review]
Patch for mouse, scroll and drag events

>+   * Return the category for the event with the given name. The name is the
>+   * event name *without* the 'on' prefix. Returns NS_EVENT if the event
>+   * doesn't match a known event name.

Some known events will return NS_EVENT as well. Some also return NS_EVENT_NULL so there doesn't seem to be a good return value. But I'd change the comment to be clearer.
Attachment #517404 - Flags: feedback?(enndeakin) → feedback+
(Assignee)

Comment 9

6 years ago
Enn, do we have any existing XBL handler modifier tests?
Don't know.
(Assignee)

Comment 11

6 years ago
Created attachment 524522 [details] [diff] [review]
With test
Assignee: nobody → neil
Attachment #517279 - Attachment is obsolete: true
Attachment #517404 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #524522 - Flags: review?(Olli.Pettay)

Updated

6 years ago
Attachment #524522 - Flags: review?(Olli.Pettay) → review+
(Assignee)

Comment 12

6 years ago
Pushed changeset c65c39ca8b11 to mozilla-central.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Blocks: 650950

Updated

6 years ago
Blocks: 659968
You need to log in before you can comment on or make changes to this bug.