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)
Core
DOM: Events
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
Reporter | ||
Comment 1•8 years ago
|
||
(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
Reporter | ||
Updated•8 years ago
|
Priority: -- → P3
Comment 2•8 years ago
|
||
Btw, could you update the Gecko profiler addon?
https://perf-html.io/
Comment 3•8 years ago
|
||
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?
Reporter | ||
Comment 4•8 years ago
|
||
(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.
Reporter | ||
Comment 5•8 years ago
|
||
Comment 6•8 years ago
|
||
ok, that hints we're in active drag-and-drop session waiting for more messages from OS
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → sshih
Assignee | ||
Comment 7•8 years ago
|
||
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
Assignee | ||
Comment 8•8 years ago
|
||
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?
Assignee | ||
Comment 9•8 years ago
|
||
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)
Assignee | ||
Comment 10•8 years ago
|
||
Reporter | ||
Comment 11•8 years ago
|
||
(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)
Comment 12•8 years ago
|
||
We can of course internally store tiltX/Y also in mouse events, if that helps dispatching proper pointer events.
Assignee | ||
Comment 13•8 years ago
|
||
(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)
Assignee | ||
Comment 14•8 years ago
|
||
Attachment #8849443 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Attachment #8849873 -
Flags: review?(jmathies)
Comment 15•8 years ago
|
||
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+
Assignee | ||
Comment 16•8 years ago
|
||
Attachment #8849873 -
Attachment is obsolete: true
Attachment #8854323 -
Flags: review+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 17•8 years ago
|
||
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
Comment 18•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment 19•7 years ago
|
||
hmm, so this broke all pen handling :/
Comment 20•6 years ago
|
||
In response to comment 19, I've logged this as a new bug https://bugzilla.mozilla.org/show_bug.cgi?id=1487509
You need to log in
before you can comment on or make changes to this bug.
Description
•