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

Status: VERIFIED FIXED

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

Description

[Avi Halachmi (:avih)]

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.

Comment 1

[:Felipe Gomes (needinfo for replies!)]

Attachment: 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.

Comment 2

[Jim Mathies [:jimm]]

(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.

Comment 3

[Avi Halachmi (:avih)]

(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?

Comment 4

[Jim Mathies [:jimm]]

(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.

Comment 5

[Avi Halachmi (:avih)]

(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.

Comment 6

[:Felipe Gomes (needinfo for replies!)]

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

Comment 7

[Avi Halachmi (:avih)]

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?

Comment 8

[:Gavin Sharp [email: [e-mail]]]

(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.

Comment 9

[Avi Halachmi (:avih)]

(In reply to :Gavin Sharp (use [e-mail] 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)?

Comment 10

[Avi Halachmi (:avih)]

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.

Comment 11

[:Felipe Gomes (needinfo for replies!)]

(In reply to :Gavin Sharp (use [e-mail] 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).

Comment 12

[:Gavin Sharp [email: [e-mail]]]

(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 13

[Jim Mathies [:jimm]]

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.

Comment 14

[stevesliva]

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 15

[:Gavin Sharp [email: [e-mail]]]

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.

Comment 16

[:Felipe Gomes (needinfo for replies!)]

Landed in fx-team, which is now being frequently merged to m-c like inbound:
https://hg.mozilla.org/integration/fx-team/rev/04ec1187385c

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/04ec1187385c

Comment 19

[Jim Mathies [:jimm]]

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

Comment 20

[Lukas Blakk [:lsblakk] use ?needinfo]

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.

Comment 21

[Jim Mathies [:jimm]]

Attachment: export for aurora

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a776e6dd95c9

Comment 23

[Avi Halachmi (:avih)]

Thanks :)

Comment 24

[Kohei Yoshino [:kohei]]

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

Comment 25

[:Gavin Sharp [email: [e-mail]]]

Avi/Felipe, can you also post to dev.platform with a short summary of the rationale, to give people a heads up about this?

Comment 26

[Avi Halachmi (:avih)]

Good idea. I'll do that.

Comment 27

[Avi Halachmi (:avih)]

Any reason for the flags modification?

Comment 28

[:Gavin Sharp [email: [e-mail]]]

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.

Comment 29

[Avi Halachmi (:avih)]

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.

Comment 30

[u279076]

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?

Comment 31

[:Felipe Gomes (needinfo for replies!)]

It should restore touch scrolling

Comment 32

[u279076]

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?

Comment 33

[u279076]

Sorry, I need to retest this. Somehow the Aurora build I download was an old build (24.0a2 20130805).

Comment 34

[u279076]

(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.

Comment 35

[ruben.nesvadba]

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)

Comment 36

[Kevin Brosnan [Ex-Mozilla]]

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

This is not an end user feature so it is not appropriate for release notes.

Comment 37

[Jim Mathies [:jimm]]

(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.

Comment 38

[Kohei Yoshino [:kohei]]

(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

Comment 39

[james.setaro]

How is this fixed when it doesn't work on firefox desktop?  More and more touch supported desktop devices are coming out each year.

Comment 40

[romikette]

+1 
it's only been 3 months since james pointed out it's still not working;

Comment 41

[Daniel Holbert [:dholbert]]

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