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)

Unspecified
Windows 8.1
defect

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)

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.
(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?
Component: Untriaged → General
Yeah, this was working on a recent nightly (build 20150601075320). It's broken for me *everywhere* now.
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
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
Thanks everyone!
IMHO this bug should be reopened for finding correct solution for resolving this bug.
Summary: Touch Scrolling No Longer Works → Touch Scrolling No Longer Works after enabling Pointer Events
Attached patch scrolling_issue_ver1.diff (obsolete) (deleted) — Splinter Review
- 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)
Attachment #8615420 - Flags: review?(bugmail.mozilla)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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)
Why were if (gIsPointerEventsEnabled) { checks added to nsWindow.cpp originally?
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-
To support gesture with touch event, we have to support Direct Manipulation API (bug 890878).
(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.
Attached patch scrolling_issue_ver2.diff (deleted) — Splinter Review
- 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)
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 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+
(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)
Flags: needinfo?(sdwilsh)
(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.
Attachment #8615928 - Flags: feedback?(jmathies) → feedback+
Assignee: nobody → alessarik
If there are no objections, I put checkin-needed flag.
Keywords: checkin-needed
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Flags: qe-verify+
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.

Attachment

General

Creator:
Created:
Updated:
Size: