Closed Bug 1345398 Opened 8 years ago Closed 8 years ago

Browser is hung when I long press a link with a stylus with PointerEvents enabled

Categories

(Core :: DOM: Events, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: hsinyi, Assigned: stone)

References

(Depends on 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

I don't have strict steps to reproduce, but basically when I use a stylus to long press a link, it's very easy that I run into the situation that firefox browser is hung. Profiling data: https://cleopatra.io/#report=37c0ca80c7efe0383a44227bf6b12d2ee83a774b&selection=0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #0) > I don't have strict steps to reproduce, but basically when I use a stylus to > long press a link, it's very easy that I run into the situation that firefox > browser is hung. > > Profiling data: > https://cleopatra.io/ > #report=37c0ca80c7efe0383a44227bf6b12d2ee83a774b&selection=0,1,2,3,4,5,6,7,8, > 9,10,11,12,13,14,15,16 b.t.w. I was using Surface pro on Win10, Nightly 55
Priority: -- → P3
Btw, could you update the Gecko profiler addon? https://perf-html.io/
Hmm, I don't quite understand that profile. It has several Gecko Main processes. But anyhow, the topmost seems to be the parent one and it is waiting for input from OS. Is this some widget/windows level issue?
(In reply to Olli Pettay [:smaug] (high review load, queue closed for couple of days) from comment #2) > Btw, could you update the Gecko profiler addon? > https://perf-html.io/ (In reply to Olli Pettay [:smaug] (high review load, queue closed for couple of days) from comment #3) > Hmm, I don't quite understand that profile. It has several Gecko Main > processes. > But anyhow, the topmost seems to be the parent one and it is waiting for > input from OS. > Is this some widget/windows level issue? Sure, let me update the addon and get another profile.
ok, that hints we're in active drag-and-drop session waiting for more messages from OS
Assignee: nobody → sshih
According to [1], we have to start windows native dnd (invocation of ::DoDragDrop) in the Windows mouse messages handler. Windows native drag&drop is not supported in the touch and pen message handler. The current implementation fires mouse events when handling WM_POINTER* because we only can fetch pen attributes from pointer messages. Maybe we could fetch pen attributes from pointer messages and cache them, then use them to dispatch mouse events by handling Windows mouse messages. [1] https://msdn.microsoft.com/zh-tw/library/windows/desktop/ms678486(v=vs.85).aspx
Oops. Not all pointer messages and their corresponding pointer messages are interleaved. Can't easily cache the attributes from the pointer message and use it to dispatch mouse events when handling the corresponding pointer message. Maybe we could refer to Edge's behavior and disable dnd when dragging with pen?
After more investigations, there are some tradeoffs when handling and consuming WM_POINTER* for pen. 1. dnd is not supported in the pen/touch message handler 2. WM_CONTEXTMENU is also consumed If we handle Windows mouse messages, then we can't get some attributes (e.g. tiltX, tiltY) If we handle Windows pointer messages for pointer event and Windows mouse messages for compatibility mouse events, we might have to change more logic since the pointer messages and mouse messages are not one to one mapping. At this moment, I prefer the option to cache attributes from WM_POINTER* for pen and use them to fire WidgetMouseEvent by handling Windows mouse messages. Considering that we might move to WM_POINTER* someday, plan to use a preference to control handling mouse messages and pointer messages, and handle mouse messages by default. Wondering if HsinYi has some suggestions/thoughts about this?
Flags: needinfo?(htsai)
(In reply to Ming-Chou Shih [:stone] from comment #9) > After more investigations, there are some tradeoffs when handling and > consuming WM_POINTER* for pen. > 1. dnd is not supported in the pen/touch message handler > 2. WM_CONTEXTMENU is also consumed > > If we handle Windows mouse messages, then we can't get some attributes (e.g. > tiltX, tiltY) Without these attributes, will certain websites in the list that SV tested turn to not work? Do you know if this is the way chrome does? > > If we handle Windows pointer messages for pointer event and Windows mouse > messages for compatibility mouse events, we might have to change more logic > since the pointer messages and mouse messages are not one to one mapping. > > At this moment, I prefer the option to cache attributes from WM_POINTER* for Is WM_POINTER a typo? Reading the context, looks like you prefer handling mouse events for PE? > pen and use them to fire WidgetMouseEvent by handling Windows mouse messages. > > Considering that we might move to WM_POINTER* someday, plan to use a > preference to control handling mouse messages and pointer messages, and > handle mouse messages by default. > > Wondering if HsinYi has some suggestions/thoughts about this Any idea of how much effort is needed for each one? I suppose using Windows mouse messages would be a quite small task, and the other needs more complicated handling, right? From the priority and resource point of view, I'd suggest we take the way that needs little time and the impact is much narrower and manageable. Thus, Windows mouse messages look a better option for me though I'd like to hear from you about the comments above. :)
Flags: needinfo?(htsai) → needinfo?(sshih)
We can of course internally store tiltX/Y also in mouse events, if that helps dispatching proper pointer events.
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #11) > Without these attributes, will certain websites in the list that SV tested > turn to not work? > Do you know if this is the way chrome does? The tiltX and tiltY are always 0 on Chrome. Edge does show tiltX and tiltY and it doesn't support dnd with pen. > Is WM_POINTER a typo? Reading the context, looks like you prefer handling > mouse events for PE? We can fetch and cache tiltX and tiltY from WM_POINTER* but not dispatch WidgetMouseEvent. When handling Windows mouse messages, we use the cached tiltX and tiltY to dispatch WidgetMouseEvent. > Any idea of how much effort is needed for each one? Handle Windows mouse messages and set tiltX and tiltY to zero --> Rollback part1 of bug 1031362 Fetch and cache attributes from WM_POINTER* and use them to fire WidgetMouseEvent when handling Windows mouse messages --> attachment 8849443 [details] [diff] [review]
Flags: needinfo?(sshih)
Attachment #8849873 - Flags: review?(jmathies)
Comment on attachment 8849873 [details] [diff] [review] Fire WidgetMouseEvent by handling pen generated mouse messages to get Windows native dnd supports (V2) Review of attachment 8849873 [details] [diff] [review]: ----------------------------------------------------------------- ::: modules/libpref/init/all.js @@ +4906,5 @@ > pref("dom.w3c_pointer_events.enabled", false); > #endif > > +#if defined(XP_WIN) > +pref("dom.w3c_pointer_events.dispatch_by_pointer_messages", false); nit - comment explaining what this pref is about.
Attachment #8849873 - Flags: review?(jmathies) → review+
Keywords: checkin-needed
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/22813d104a9a Fire WidgetMouseEvent by handling pen generated mouse messages to get Windows native dnd supports. r=jimm
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
hmm, so this broke all pen handling :/
In response to comment 19, I've logged this as a new bug https://bugzilla.mozilla.org/show_bug.cgi?id=1487509
Depends on: 1487509
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: