369793 - DOMMouseScroll events not dispatched when they cause text zoom changes

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

nsEventStateManager::DoScrollTextsize doesn't dispatch events, so when nsEventStateManager::PostHandleEvent calls it to change the text zoom, event listeners don't get notified about the DOMMouseScroll event.

Contrast with nsEventStateManager::DoScrollText, which does dispatch events.  Seems like DoScrollTextsize should dispatch events just like DoScrollText does, so listeners who care about text zoom changes can find out about them.

Comment 1

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

Attachment: a testcase

The testcase shows that currently DOMMouseScroll is also targeted wrongly.Event target shouldn't be [object Text], which it is currently
if mouse is over some text.

Comment 2

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

Attachment: Make DOMMouseScroll work more like other events

This removes explicit DOMMouseScroll dispatching, because there is 
already NS_MOUSE_SCROLL event being dispatched. 
event->detail is now updated in ESM::PreHandleEvent so that it has
the right value during the actual event dispatching.

Comment 3

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

Comment on attachment 254795 [details] [diff] [review]
Make DOMMouseScroll work more like other events

Oops, this doesn't handle nsIDOMNSUIEvent::SCROLL_PAGE_DOWN
properly.

Comment 4

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

Attachment: handles nsIDOMNSUIEvent::SCROLL_PAGE_UP/DOWN

This adds 2 calls to Get***Pref comparing to the current code,
but mouse wheel events aren't
dispatched that often (comparing to mouse move, for example).
The reason for those 2 calls is that PreHandleEvent needs to know 
whether to useSysNumLines and what is the action for the mousewheel event. Those same values are then needed in PostHandleEvent.

The patch fixes the event targeting bug, because DOMMouseScroll is now
handled like any other event.

Comment 5

[Boris Zbarsky [:bzbarsky]]

I'm not going to be able to review this quickly... Please ask someone else if you want a review in the next few weeks?

Comment 6

[Johnny Stenback (:jst)]

Comment on attachment 254810 [details] [diff] [review]
handles nsIDOMNSUIEvent::SCROLL_PAGE_UP/DOWN

r=jst

Comment 7

[Peter Van der Beken [:peterv]]

Comment on attachment 254810 [details] [diff] [review]
handles nsIDOMNSUIEvent::SCROLL_PAGE_UP/DOWN

I wonder if it wouldn't be cheaper to use AppendLiteral instead of NS_LITERAL_CSTRING/Append. Either way, sr=peterv.

Comment 8

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

Writing a mochitest would require (an enhanced version of ) bug 364508.