Closed
Bug 1171101
Opened 9 years ago
Closed 9 years ago
Touch Scrolling No Longer Works after enabling Pointer Events
Categories
(Firefox :: General, defect, P1)
Tracking
()
VERIFIED
FIXED
Firefox 41
Tracking | Status | |
---|---|---|
firefox41 | --- | verified |
People
(Reporter: sdwilsh, Assigned: alessarik)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
smaug
:
review+
jimm
:
feedback+
|
Details | Diff | Splinter Review |
I updated to Nightly build 20150602055237, and now I can no longer touch a page in an area without text (on my Surface) and drag to scroll. It just selects text.
I see this on my Venue 8 Pro tablet, too, so it's not just the Surface hardware.
Comment 2•9 years ago
|
||
(ohai!)
Were you running a recent previous Nightly? I just noticed this yesterday on a new Surface Pro 3, and was confused because I thought it working the day before (but it being new, hadn't tested it extensively). Sounds like this might actually be a recent regression.
IRC conversation:
[16:38] <dolske> jaws: hum, I can't seem to touch-scroll pages in Nightly all of a sudden, it just selects
text. I don't think anything just changed in FF, is this some weird Windows mode?
[16:38] <dolske> wfm in chrome and edge. O_o
[16:39] <jaws> dolske: touch-scroll wfm in 2015-06-01
[16:45] <mbrubeck> dolske: It's long been kinda janky; it depends on the initial direction of finger motion
for example.
[16:45] <mbrubeck> and possibly on the page content
[16:46] <dolske> mbrubeck: hmm. is there a way to get out of it?
[16:46] <dolske> at the moment I have a devmo page I can't scroll up or down in
[16:47] <dolske> maybe it's the content? I only tested a couple other pages since noticing it.
[16:47] <mbrubeck> dolske: Right now IIRC we don't actually implement touch scrolling on Windows; we rely on
the OS translating touch gestures into either scroll or mouse events based on some
heuristics for non-touch-aware apps.
[16:47] <mbrubeck> This'll change once we enable APZC and touch events on Windows, then we should have
"real" touch scrolling support, I believe.
[16:47] <dolske> yikes
[16:48] <dolske> mbrubeck: happen to have a bug #?
[16:50] <mbrubeck> dolske: There are a lot of related bugs; looking to see if there's a good central one...
[16:51] <dolske> now I'm wondering if I just didn't notice this because I was focusing on trying out Edge,
or if it was working for me earlier but is now broken :/
[16:51] <mbrubeck> dolske: Most directly related https://bugzilla.mozilla.org/show_bug.cgi?id=736048
[16:51] <firebot> Bug 736048 — NEW, nobody@mozilla.org — Some sites won't touch-scroll using a touch-screen
[16:52] <mbrubeck> dolske: Since we killed metrofox, I've mostly been using IE for browsing on my Windows
slate. :/
[16:52] * dolske reboots, still wonky. huh.
[16:52] <dolske> yeah, this makes touch pretty much unusuable.
[16:52] <dolske> phlsa_away: ^ were you seeing this too?
[17:04] <shorlander-away> dolske: I can't get any text selection
[23:50] <phlsa> dolske: Touch performance is bad and it often triggers incorrectly (sometimes text gets
selected, sometimes the page scrolls)
[23:50] <phlsa> dolske: Happens more often on some sites (IRCcloud is really bad for me) than on others
[23:51] <phlsa> dolske: But I haven't encountered a site where it just wouldn't work at all
[23:51] <phlsa> dolske: Did we have a solution for that in metrofox?
Updated•9 years ago
|
Component: Untriaged → General
Reporter | ||
Comment 3•9 years ago
|
||
Yeah, this was working on a recent nightly (build 20150601075320). It's broken for me *everywhere* now.
Comment 4•9 years ago
|
||
Yeah, broken between 6/2 and 6/3 with mozregression (which isn't helping me bisect into mozilla-inbound, alas)
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=666b584fb521&tochange=9eae3880b132
Bug 1166347 looks likely, and indeed disabling dom.w3c_pointer_events.enabled makes touch scrolling work again. I think we should either back that out, or only enable pointer events on non-Windows.
Blocks: 1166347
Priority: -- → P1
Updated•9 years ago
|
Blocks: windows-10
Reporter | ||
Comment 5•9 years ago
|
||
I can also confirm that disabling pointer events makes the problem go away.
I backed out the patch from bug 1166347 in https://hg.mozilla.org/mozilla-central/rev/a8edd0679b57 which will be in tonight's Nightly update.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 7•9 years ago
|
||
Thanks everyone!
Assignee | ||
Comment 8•9 years ago
|
||
IMHO this bug should be reopened for finding correct solution for resolving this bug.
Assignee | ||
Updated•9 years ago
|
Summary: Touch Scrolling No Longer Works → Touch Scrolling No Longer Works after enabling Pointer Events
Assignee | ||
Comment 9•9 years ago
|
||
- Removed relations between pointer events and scrolling via gestures
Suggestions and comments and objections are very welcome.
Attachment #8615420 -
Flags: review?(bugs)
Attachment #8615420 -
Flags: review?(bugmail.mozilla)
Attachment #8615420 -
Flags: feedback?(jmathies)
Updated•9 years ago
|
Attachment #8615420 -
Flags: review?(bugmail.mozilla)
Updated•9 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 10•9 years ago
|
||
TRY: https://treeherder.mozilla.org/#/jobs?repo=try&revision=edfc95767c95
Could anybody confirm that patch resolves issue with enabling Pointer Events ?
Flags: needinfo?(v-dmbark)
Flags: needinfo?(sdwilsh)
Comment 11•9 years ago
|
||
Why were if (gIsPointerEventsEnabled) { checks added to
nsWindow.cpp originally?
Comment 12•9 years ago
|
||
Comment on attachment 8615420 [details] [diff] [review]
scrolling_issue_ver1.diff
Note, also Bug 736048 was regressed when pointer events were enabled.
Ah, we don't need to remove all gIsPointerEventsEnabled because
of http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsWindow.cpp?rev=e95ddbb300b2#1299
is called only when APZ is enabled.
But I think I would still prefer to change also
http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsWindow.cpp?rev=e95ddbb300b2#1312 so that we don't regress behavior when apz is enabled.
Or at least test the behavior apz+pointer events, and re-ask review.
Attachment #8615420 -
Flags: review?(bugs) → review-
Comment 13•9 years ago
|
||
To support gesture with touch event, we have to support Direct Manipulation API (bug 890878).
Assignee | ||
Comment 14•9 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #12)
> Ah, we don't need to remove all gIsPointerEventsEnabled because of
> http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsWindow.cpp?rev=e95ddbb300b2#1299
> is called only when APZ is enabled.
> But I think I would still prefer to change also
> http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsWindow.cpp?rev=e95ddbb300b2#1312
> so that we don't regress behavior when apz is enabled.
> Or at least test the behavior apz+pointer events, and re-ask review.
As I understand correctly check for APZ happens only at start. So if APZ disabled we should use gestures in any case (I mean without check for pointer events).
And as I understand correctly we can use only gestures or only touch events, so we should not add any cross checks. (I mean using flag mTouchWindow as cross checks)
But remove check for pointer events from nsWindow::RegisterTouchWindow seems reasonable for me.
As result after that we can only check such configuration:
E10S(enabled) + APZ(enabled) + PointerEvents(enabled) + mTouchWindow(disabled) + Gesture(enabled)
Because all other configurations looks obvious and were tested earlier.
Assignee | ||
Comment 15•9 years ago
|
||
- Remove pointer event check from RegisterTouchWindow
Suggestions and comments and objections are very welcome.
Attachment #8615420 -
Attachment is obsolete: true
Attachment #8615420 -
Flags: feedback?(jmathies)
Attachment #8615928 -
Flags: review?(bugs)
Attachment #8615928 -
Flags: feedback?(jmathies)
Assignee | ||
Comment 16•9 years ago
|
||
TRY: https://treeherder.mozilla.org/#/jobs?repo=try&revision=04ff691f29d4
So touch events dependence from pointer events was removed, that's why all configurations will be:
e10s(True/False) + APZ(True/False) + TouchesEvents(True/False) + PointerEvents(True/False)
Comment 17•9 years ago
|
||
Comment on attachment 8615928 [details] [diff] [review]
scrolling_issue_ver2.diff
Yeah, this is safer, and please file then a new bug to sort out touch handling when pointer events are enabled.
Attachment #8615928 -
Flags: review?(bugs) → review+
Comment 18•9 years ago
|
||
(In reply to Maksim Lebedev from comment #10)
> TRY: https://treeherder.mozilla.org/#/jobs?repo=try&revision=edfc95767c95
> Could anybody confirm that patch resolves issue with enabling Pointer Events
> ?
Tested on Windows 8.1 seems scroll starts working with layout.css.touch_action enabled, dom.w3c_pointer_events enabled and layers.async-pan-zoom disabled.
Also performed some smoke testing including pen, touch and mouse and haven't found critical issues (i.e. user can use browser with pointer events enabled and pan-zoom disabled).
Flags: needinfo?(v-dmbark)
Reporter | ||
Updated•9 years ago
|
Flags: needinfo?(sdwilsh)
Comment 20•9 years ago
|
||
(In reply to Maksim Lebedev from comment #16)
> TRY: https://treeherder.mozilla.org/#/jobs?repo=try&revision=04ff691f29d4
> So touch events dependence from pointer events was removed, that's why all
> configurations will be:
> e10s(True/False) + APZ(True/False) + TouchesEvents(True/False) +
> PointerEvents(True/False)
I've checked all combinations of the settings above. Scrolling is working on each combination and have similar behaviour. Enabling or disabling each of this settings individually or in combination doesn't spoil user browsing expirience.
Updated•9 years ago
|
Attachment #8615928 -
Flags: feedback?(jmathies) → feedback+
Updated•9 years ago
|
Assignee: nobody → alessarik
Assignee | ||
Comment 21•9 years ago
|
||
If there are no objections, I put checkin-needed flag.
Keywords: checkin-needed
Comment 22•9 years ago
|
||
Keywords: checkin-needed
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Updated•9 years ago
|
Flags: qe-verify+
Comment 24•9 years ago
|
||
Reproduced with Nightly from June 2nd (BuildID=20150602055237), on a Dell Ultrabook using Win 10 x64. The issue no longer reproduced on Latest Firefox Nightly (BuildID=20150616030201).
Touch scrolling works in all combinations of the following preferences: e10s(True/False) + APZ(True/False) + TouchesEvents(True/False) + PointerEvents(True/False).
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•