639338 - Unable to specify XBL modifiers for all possible mouse events

Status: RESOLVED FIXED

(Core :: XBL, defect)

Description

[[e-mail]]

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.

Comment 1

[[e-mail]]

Attachment: Patch for DOMMouseScroll only

Comment 2

[Neil Deakin]

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.

Comment 3

[[e-mail]]

(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.

Comment 4

[Neil Deakin]

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

[Olli Pettay [:smaug][[e-mail]]]

(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

Comment 6

[[e-mail]]

Attachment: Patch for mouse, scroll and drag events

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

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.

Comment 8

[Neil Deakin]

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.

Comment 9

[[e-mail]]

Enn, do we have any existing XBL handler modifier tests?

Comment 10

[Neil Deakin]

Don't know.

Comment 11

[[e-mail]]

Attachment: With test

Comment 12

[[e-mail]]

Pushed changeset c65c39ca8b11 to mozilla-central.