Closed Bug 888304 Opened 11 years ago Closed 11 years ago

Content touch-events on Firefox-desktop should be disabled until we can support them properly

Categories

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

x86_64
Windows 7
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla25
Tracking Status
firefox24 + verified
firefox25 + verified
relnote-firefox --- 24+

People

(Reporter: avih, Assigned: Felipe)

References

Details

(Keywords: dev-doc-complete, site-compat)

Attachments

(2 files)

Until bug 736048 is fixed, some very high profile pages (google search results, twitter, some MDN pages) cannot be scrolled using touch, and instead text-selection is invoked. See Bug 736048 comment 30.

It would probably be better to disable content-touch completely until bug 736048 is fixed.
Attached patch PatchSplinter Review
metro.js defines this pref to 1 which I believe will override what's defined in firefox.js. is this correct? if so this should work properly.
Assignee: nobody → felipc
Status: NEW → ASSIGNED
Attachment #769089 - Flags: review?(jmathies)
(In reply to :Felipe Gomes from comment #1)
> Created attachment 769089 [details] [diff] [review]
> Patch
> 
> metro.js defines this pref to 1 which I believe will override what's defined
> in firefox.js. is this correct? if so this should work properly.

two different browsers, firefox.js isn't used by metrofx.


I'm not sure about disabling these. They have been on since last fall I think? Rather than disable the events, we should just fix the bugs.
(In reply to Jim Mathies [:jimm] from comment #2)
> 
> I'm not sure about disabling these. They have been on since last fall I
> think? Rather than disable the events, we should just fix the bugs.

Bug 736048 might be tolerable on nightly/aurora, but IMO beta and release shouldn't suffer from it. Not being able to scroll the google results page is not fun.

From email correspondence with Asa:

avih:
> Fixing this properly could take a while. I suggest to disable content-touch until
> we could properly support it, such that until then those pages could still be
> scrolled by touch (even if it costs us to not support native page-touch-handling).

Asa:
> I have no problem disabling touch this on desktop, but we have to fix
> these things for Metro. Is the code radically different?
(In reply to Avi Halachmi (:avih) from comment #3)
> (In reply to Jim Mathies [:jimm] from comment #2)
> > 
> > I'm not sure about disabling these. They have been on since last fall I
> > think? Rather than disable the events, we should just fix the bugs.
> 
> Bug 736048 might be tolerable on nightly/aurora, but IMO beta and release
> shouldn't suffer from it.

Support has been on release since January, the original patches landed back in September of last year.

> Not being able to scroll the google results page is not fun.

I'm not sure what's wrong with google's search results page. If it is cuased by our touch implementation, all the more reason to find the time to finish up bug 736048.

Note, only about 4% of our user base has touch capable hardware. We also expose the pref if users really want to turn touch events off. That's not an excuse to put off the work however. I don't think it will require a herculean effort either so we just need to find the time to do the work. I took that bug a few weeks ago with the intent to work on it. If nobody else picks it up I'll get to it fairly soon.
(In reply to Jim Mathies [:jimm] from comment #4)
> > Not being able to scroll the google results page is not fun.
>
> Note, only about 4% of our user base has touch capable hardware. We also
> expose the pref if users really want to turn touch events off.

Using the pref to disable content-touch support is kinda broken. Bug 888300 was filed today to fix it. The patch was tested locally and appears to allow properly disabling w3c touch events (which makes the results page touch-scrollable again).

Also, I just tested the same google results page with a windows 7 tablet and Firefox 22 release and Firefox 25 nightly, and touch-scroll of this page is broken with both (selects instead of scrolls).

While it may only affect 4% of our users, 100% of those who use google were not able to touch-scroll google search results page for quite a while, and will not be able to do so until bug 736048 lands and reaches release.

Disabling w3c touch events by default and possibly uplifting it a bit could be a reasonable stop-gap for a relatively major issue for those users.

There's no absolute right or wrong here. Someone just needs to take a decision.
Independent of the decision if we should switch this or not, the risk for making this change is minimal:

- the main concern for this type of change is of sites relying on the existence of these events to do browser detection. However, for desktop Firefox, the current value of this pref means that the events are only exposed to Windows 7/8 users who actually have a touch device. Then, for the majority of users, these events were already not exposed, and then it's very unlikely that sites would be relying on that to do any type of browser detection

- Obviously, sites that are specifically tailored for touch and were only accessible by these users will be unable to use these events and will have to fallback to the desktop version

- This doesn't change the behavior on Metro where touch will always be exposed
Thanks felipe.

Following comment 4 which suggests to WONTFIX this bug and instead go ahead and fix bug 736048, I think we need a product decision on this. Asa?

Summary:
1. On Firefox-desktop, on google results page and other high profile pages, the page cannot be scrolled using touch - it selects text instead.
2. Disabling w3c touch events "fixes" this problem, but also prevents pages from handling touch events. Touch-Scroll would work, but otherwise pages will not utilize touch.
3. Other than the obvious result of pages not being able to utilize touch, current assessment at comment 6 is that this and bug 888300 are low risk.

needinfo:
- Should we disable w3c touch by default until bug 736048 is fixed?
- Should we uplift this bug and bug 888300 (which fixes respecting the pref)? up to beta?
Flags: needinfo?(asa)
(In reply to Avi Halachmi (:avih) from comment #7)
> 1. On Firefox-desktop, on google results page and other high profile pages,
> the page cannot be scrolled using touch - it selects text instead.

Since when? I.e. when did we introduce this bug?

> 2. Disabling w3c touch events "fixes" this problem, but also prevents pages
> from handling touch events.

Pages could only ever possibly handle touch events on desktop for Win8 touch-enabled devices, right?

> - Should we disable w3c touch by default until bug 736048 is fixed?

Seems reasonable, assuming my understanding above is correct.

> - Should we uplift this bug and bug 888300 (which fixes respecting the
> pref)? up to beta?

Depends on the "since when" question, I think.
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #8)
> (In reply to Avi Halachmi (:avih) from comment #7)
> > 1. On Firefox-desktop, on google results page and other high profile pages,
> > the page cannot be scrolled using touch - it selects text instead.
> 
> Since when? I.e. when did we introduce this bug?

I've seen it on nightlies since I put my hands on a touch device on May, and the bug also exists in current release FX22. According to this, since September:

(In reply to Jim Mathies [:jimm] from comment #4)
> > Bug 736048 might be tolerable on nightly/aurora, but IMO beta and release
> > shouldn't suffer from it.
> 
> Support has been on release since January, the original patches landed back
> in September of last year.


> Pages could only ever possibly handle touch events on desktop for Win8
> touch-enabled devices, right?

Also Windows 7 tablets. I guess they're quite rare, but as of few weeks ago jlin had the one we used to test this with at the Toronto workweek (he specifically installed win7 touch on it for our test).


> > - Should we uplift this bug and bug 888300 (which fixes respecting the
> > pref)? up to beta?
> 
> Depends on the "since when" question, I think.

For the reason that it might not be broken in beta (it is broken in beta)? or that it's been broken long enough that it doesn't matter anymore and we should just fix it properly instead (and wait till it reaches release)?
Also, we might want to add a regression test for this. AFAIK we can synthesize touch events, so it should be possible to make sure that we can touch-scroll on cases which matter to us.
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #8)
> (In reply to Avi Halachmi (:avih) from comment #7)
> > 1. On Firefox-desktop, on google results page and other high profile pages,
> > the page cannot be scrolled using touch - it selects text instead.
> 
> Since when? I.e. when did we introduce this bug?

The timeline is not in a regression from our code, our touch/scroll handling has always been this way. The difference is that, at some point, google.com introduced a TouchEvent listener on the page, which puts our handling in non-scroll mode (which will do text selection).
(In reply to Avi Halachmi (:avih) from comment #9)
> For the reason that it might not be broken in beta (it is broken in beta)?
> or that it's been broken long enough that it doesn't matter anymore and we
> should just fix it properly instead (and wait till it reaches release)?

The latter. Since we've been shipping this for a while and apparently few people have noticed, the urgency to uplift a behavior change seems low.

I think we should move forward and do this, though.
Comment on attachment 769089 [details] [diff] [review]
Patch

I can approve this patch functionally. I'd prefer a driver sign off on the product decision.
Attachment #769089 - Flags: superreview?(asa)
Attachment #769089 - Flags: review?(jmathies)
Attachment #769089 - Flags: review+
FWIW, Win8 Firefox 22 with netvibes.com (my personal Google Reader replacement) was *terrible* before I set dom.w3c_touch_events.enabled to 0.

I have a convertible laptop.  I want it to be a tablet to plow through my feeds (lots of text) with a touchscreen and no keyboard.  This bug made my preferred website utterly useless without a mouse hooked up, which also means the tablet configuration gets lamely chained to a desk.

I think you're right that the users who need this now is small, but if they're anything like me their next purchase will have a touchscreen.

If you need another tester for future patches, I volunteer.  Email me.  Win 8, Lenovo Twist.  Thanks!
Comment on attachment 769089 [details] [diff] [review]
Patch

I spoke to Avi on IRC, let's go ahead and do this.

Asa, if you have an objection we can discuss it further, but in the meantime we can get some Nightly feedback.
Attachment #769089 - Flags: superreview?(asa)
Landed in fx-team, which is now being frequently merged to m-c like inbound:
https://hg.mozilla.org/integration/fx-team/rev/04ec1187385c
https://hg.mozilla.org/mozilla-central/rev/04ec1187385c
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Comment on attachment 769089 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): No bug
User impact if declined: touch support we want disabled stays enabled longer in release channel.
Testing completed (on m-c, etc.): yes
Risk to taking this patch (and alternatives if risky): sites that have come to expect our touch input support on desktop break. But they are going to break ultimately as this patch rolls out.
String or IDL/UUID changes made by this patch: none
Attachment #769089 - Flags: approval-mozilla-beta?
Attachment #769089 - Flags: approval-mozilla-aurora?
Comment on attachment 769089 [details] [diff] [review]
Patch

This is an old issue so we won't take it on Beta (which has already gone to RC for FF23) but let's get it on Aurora and disable there at least.
Attachment #769089 - Flags: approval-mozilla-beta?
Attachment #769089 - Flags: approval-mozilla-beta-
Attachment #769089 - Flags: approval-mozilla-aurora?
Attachment #769089 - Flags: approval-mozilla-aurora+
Keywords: checkin-needed
Thanks :)
Avi/Felipe, can you also post to dev.platform with a short summary of the rationale, to give people a heads up about this?
Good idea. I'll do that.
Any reason for the flags modification?
Flags: needinfo?(akeybl)
tracking? so that these can be triaged and discussed in the triage meetings, presumably.

relnote-firefox because this is probably something we should add to the release notes.
Thanks, I misread the changes.

Also, it should be noted that bug 888300 still awaits approval (now for beta 24), and without it this bug will not be effective since the pref will not be respected properly.
Flags: needinfo?(akeybl)
I'd like to test this to confirm touch is disabled in the latest Aurora and Beta builds. Should this patch restore touch scrolling functionality to those regressed sites or should touch not work at all at this point?
It should restore touch scrolling
Firefox 24.0b6 now scrolls on Google search results and MDN pages.
Firefox 25.0a2 20130828 still does text selection instead of scrolling. Did this make it into Aurora?
Sorry, I need to retest this. Somehow the Aurora build I download was an old build (24.0a2 20130805).
(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #33)
> Sorry, I need to retest this. Somehow the Aurora build I download was an old
> build (24.0a2 20130805).

This is bug 910442 -- stub installer is staging an old Aurora build right now.

I did manage to get the latest Aurora build though and do confirm that the scrolling functionality has returned. Marking this bug verified fixed.
Status: RESOLVED → VERIFIED
Why don't the release notes for Firefox 24 final mention this change?
The release notes for Firefox 24 beta say this: 


 HTML5: Support for W3C touch events disabled (see 888304)
(In reply to Kevin Brosnan [:kbrosnan] from comment #36)
> This was documented in Comment 24.
> 
> > https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_24
> > https://developer.mozilla.org/en-US/docs/Web/Guide/DOM/Events/Touch_events

Note also that w3c touch is fully supported in metro firefox. I guess you could classify it under mobile vs. desktop.
(In reply to Kevin Brosnan [:kbrosnan] from comment #36)
> This is not an end user feature so it is not appropriate for release notes.

This bug has the relnote-firefox 24+ flag and release notes can mention developer features. It should be added to the Firefox 24 relnotes.

http://www.mozilla.org/en-US/firefox/24.0/releasenotes/

(In reply to Jim Mathies [:jimm] from comment #37)
> Note also that w3c touch is fully supported in metro firefox. I guess you
> could classify it under mobile vs. desktop.

Updated MDC docs to clarify that.

https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_24
https://developer.mozilla.org/en-US/docs/Web/Guide/API/DOM/Events/Touch_events
Flags: needinfo?(asa)
How is this fixed when it doesn't work on firefox desktop?  More and more touch supported desktop devices are coming out each year.
+1 
it's only been 3 months since james pointed out it's still not working;
Component: Event Handling → User events and focus handling

Closing the loop on this for anyone else who stumbles across this bug in the future and wonders where this ended up:

This was reenabled for Windows Nightlies in bug 1180706 comment 6, and it looks like it's now in "autodetect" mode on all platforms except for Mac (i.e. the pref is set to 2 which means "autodetect":
https://searchfox.org/mozilla-central/rev/e580b1098c1f012880368989f6e386812fe6ccc6/modules/libpref/init/StaticPrefList.yaml#4061-4072

You need to log in before you can comment on or make changes to this bug.