1166347 - Enabling pointer events in Nightly builds

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Maksim Lebedev]

Aim is enabling W3C pointer events in Nightly builds.
If anybody have objection about this action feel free to say into this bug.

Comment 1

[Maksim Lebedev]

I suggest to enable pointer events at Nightly build on Windows and Unix platform.
Touch-action can be still disabled (until enabling APZ module) or enable too.

Suggestions and comments and objections are very welcome.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Enabling on desktop should be fine (Windows/OSX/Linux). hopefully pages don't expect that
pointer events mean also touch-action is there, and vice versa. (even though touch-action isn't really useful for mouse input)
But testing this all in Nightlies should be fine.

Comment 3

[Matt Brubeck (:mbrubeck)]

I'm in favor of this.  Please set the pref to true in an "#ifdef NIGHTLY_BUILD" block, or false otherwise:

https://wiki.mozilla.org/Platform/Channel-specific_build_defines

Comment 4

[Maksim Lebedev]

Attachment: nightly_pointer_events_ver1.diff

+ Pointer events enabling at:   #if (NIGHTLY_BUILD && (XP_WIN || XP_LINUX))

Suggestions and comments and objections are very welcome.

Comment 5

[Maksim Lebedev]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=701dbb773470

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8609278 [details] [diff] [review]
nightly_pointer_events_ver1.diff

Why only Windows and Linux? Why not also OSX? (XP_MACOSX)
With that explain or the patch fixed, r+

Comment 7

[Matt Brubeck (:mbrubeck)]

Comment on attachment 8609278 [details] [diff] [review]
nightly_pointer_events_ver1.diff

I had the same question as smaug in comment 6.

Comment 8

[Maksim Lebedev]

(In reply to Olli Pettay [:smaug] from comment #6)
> Why only Windows and Linux? Why not also OSX? (XP_MACOSX)
I have no any objections about OSX.

Comment 9

[Maksim Lebedev]

Attachment: nightly_pointer_events_ver2.diff

+ Added positive preference for OSX.

Suggestions and comments and objections are very welcome.

Comment 10

[Maksim Lebedev]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b6965ff96a2d

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8610014 [details] [diff] [review]
nightly_pointer_events_ver2.diff

Why don't you use similar if-else construct as in the previous patch?
This leads to having 

pref("dom.w3c_pointer_events.enabled", false);
pref("layout.css.touch_action.enabled", false);
pref("dom.w3c_pointer_events.enabled", true);
pref("layout.css.touch_action.enabled", true);

in Nightlies, and that looks just confusing.

Comment 12

[Maksim Lebedev]

(In reply to Olli Pettay [:smaug] from comment #11)
> Why don't you use similar if-else construct as in the previous patch?
I would like to using it with pleasure, but previous try-server-build
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=701dbb773470
has a lot of errors: 'SYNTAX_ERR', 'defined(NIGHTLY_BUILD) && (defined(XP_WIN) || defined(XP_LINUX))'

Comment 13

[Kartikaya Gupta (email:[e-mail])]

I don't think the syntax in python/mozbuild/mozbuild/preprocessor.py supports parentheses. You probably need to do something like:

#if defined(NIGHTLY_BUILD) && defined(XP_WIN) || defined(NIGHTLY_BUILD) && defined(XP_LINUX) || defined(NIGHTLY_BUILD) && defined(XP_MACOSX)

Comment 14

[Kartikaya Gupta (email:[e-mail])]

(or add parentheses support to the preprocessor..)

Comment 15

[Maksim Lebedev]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #14)
> (or add parentheses support to the preprocessor..)
You have joked probably :-)

Comment 16

[Maksim Lebedev]

Attachment: nightly_pointer_events_ver3.diff

+ Added obvious #if-else statements

Suggestions and comments and objections are very welcome.

Comment 17

[Maksim Lebedev]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=81769199ab34

Comment 18

[Jim Mathies [:jimm]]

Comment on attachment 8610440 [details] [diff] [review]
nightly_pointer_events_ver3.diff

Review of attachment 8610440 [details] [diff] [review]:
-----------------------------------------------------------------

::: modules/libpref/init/all.js
@@ +4313,5 @@
>  pref("dom.w3c_touch_events.enabled", 2);
>  #endif
>  
> +#ifdef NIGHTLY_BUILD
> +#if defined(XP_WIN) || defined(XP_LINUX) || defined(XP_MACOSX)

What are we excluding here?

Comment 19

[Maksim Lebedev]

(In reply to Jim Mathies [:jimm] from comment #18)
> > +#if defined(XP_WIN) || defined(XP_LINUX) || defined(XP_MACOSX)
> What are we excluding here?
Maybe ANDROIND or B2G ...

Comment 20

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8610440 [details] [diff] [review]
nightly_pointer_events_ver3.diff

A bit ugly, but if the preprocessor just can't handle proper ifdef syntax, fine.

Comment 21

[Maksim Lebedev]

(In reply to Olli Pettay [:smaug] from comment #20)
> A bit ugly, but if the preprocessor just can't handle proper ifdef syntax, fine.
I hope in nearest future we will change it to simple PE enabling at all platforms and all builds :-)

Comment 22

[Maksim Lebedev]

If there are no objections, I will put checkin-needed flag...
...at the same time as bug 1133417 and bug 1168319.

Comment 23

[Maksim Lebedev]

If there are no objections, I put checkin-needed flag. We make a future...

Comment 24

[Carsten Book [:Tomcat]]

does this last patches fix issues from the try run like https://treeherder.mozilla.org/logviewer.html#?job_id=7886131&repo=try ?

Comment 25

[Maksim Lebedev]

(In reply to Carsten Book [:Tomcat] from comment #24)
> does this last patches fix issues from the try run like
> https://treeherder.mozilla.org/logviewer.html#?job_id=7886131&repo=try ?
That issues will be resolved at bug 1168319. Both bug have checkin-needed flag. Please, see comment 22.

Comment 26

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/5458f49a6f85

Comment 27

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5458f49a6f85

Comment 28

[Justin Dolske [:Dolske]]

So, this makes scrolling on a touchscreen device impossible (bug 1171101). Please back this out or enable it only on non-Windows (as I suspect touchscreen usage is rare on Linux, and non-existant on OS X).

Comment 29

[Olli Pettay [:smaug][[e-mail]]]

yes, we should back this out asap.

Comment 30

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out in https://hg.mozilla.org/mozilla-central/rev/a8edd0679b57 for the scrolling fail.

Comment 31

[Maksim Lebedev]

Attachment: nightly_interfaces_ver2.diff

+ PointerEvents disable:true  ->  nightly:true & desktop:true

Suggestions and comments and objections are very welcome.

Comment 32

[Maksim Lebedev]

Attachment: nightly_pointer_events_ver4.diff

+ Added explicity disabling pointer event for Android and B2G platforms


Suggestions and comments and objections are very welcome.

Comment 33

[Maksim Lebedev]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d0aba3876239

Comment 34

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8615283 [details] [diff] [review]
nightly_pointer_events_ver4.diff

Review of attachment 8615283 [details] [diff] [review]:
-----------------------------------------------------------------

I prefer what bz suggested about setting the pref to true in firefox.js only, it would be much cleaner than this.

Comment 35

[Olli Pettay [:smaug][[e-mail]]]

I prefer to not add platform specific prefs to application prefs, but don't care too much, so I can live with adding the pref to firefox.js.

Comment 36

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8615283 [details] [diff] [review]
nightly_pointer_events_ver4.diff

And since other people seems to prefer adding the 
pref("layout.css.touch_action.enabled", true);
to firefox.js, let's do that.

Comment 37

[Olli Pettay [:smaug][[e-mail]]]

Bug 1171101 must be fixed before this bug.

Comment 38

[Olli Pettay [:smaug][[e-mail]]]

or, in other words, when fixing this again, that bug must not regress.

Comment 40

[Bill Gianopoulos [:WG9s]]

This breaks Linux as well, so the maybe Linux only idea will not fly.

Comment 41

[Bill Gianopoulos [:WG9s]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #13)
> I don't think the syntax in python/mozbuild/mozbuild/preprocessor.py
> supports parentheses. You probably need to do something like:
> 
> #if defined(NIGHTLY_BUILD) && defined(XP_WIN) || defined(NIGHTLY_BUILD) &&
> defined(XP_LINUX) || defined(NIGHTLY_BUILD) && defined(XP_MACOSX)

Well if that is the case I would suggest adding a new define, #define DESKTOP that is true if it is any of the supported Desktop builds to make thinks like this both easier to implement and easier to understand looking at the code.

Comment 42

[Boris Zbarsky [:bzbarsky]]

> I prefer what bz suggested about setting the pref to true in firefox.js only

That will set it to true on Mulet, which is not considered "desktop" by test_interfaces, so will cause test failures.  You would need to either override it in b2g.js or ifdef in firefox.js.

Also, it will not set the pref for other Gecko-based things like Thunderbird and SeaMonkey.  Not sure whether that matters in practice.

Comment 44

[Maksim Lebedev]

Attachment: nightly_pointer_events_ver5.diff

+ Added explicity disabling into b2g.js
+ Moved enabling from all.js  ->  into firefox.js
+ Added explicity disabling into mobile.js

Suggestions and comments and objections are very welcome.

Comment 45

[Maksim Lebedev]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=442389e94db3

Comment 46

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8621599 [details] [diff] [review]
nightly_pointer_events_ver5.diff

Review of attachment 8621599 [details] [diff] [review]:
-----------------------------------------------------------------

Ideally you should just need to leave the prefs false in all.js, set them to true #ifdef NIGHTLY in firefox.js, and then set them back to false in mulet.js (if we care about making them false in Mulet). However I'm not sure if mulet.js actually supercedes firefox.js; I filed bug 1174234 about this. See also https://wiki.mozilla.org/Platform/Platform-specific_build_defines#Prefs_files which might be helpful.

Comment 47

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8621599 [details] [diff] [review]
nightly_pointer_events_ver5.diff

There's no point to both having b2g.js/mobile.js changes _and_ ifdefs in firefox.js.  Please pick one or the other.

Comment 48

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8621599 [details] [diff] [review]
nightly_pointer_events_ver5.diff

yeah, just enable the prefs in firefox.js and don't have any ifdefs there.

Comment 49

[Olli Pettay [:smaug][[e-mail]]]

And since the prefs are false by default, just have the prefs enabled in firefox.js

Comment 50

[Boris Zbarsky [:bzbarsky]]

> And since the prefs are false by default, just have the prefs enabled in firefox.js

That will make tests fail in mulet, which loads firefox.js _and_ b2g.js.  So you need to redisable in b2g.js if you want tests to test that they're disabled on b2g.

Comment 51

[Olli Pettay [:smaug][[e-mail]]]

Oh, right. That is silly.

Comment 52

[Maksim Lebedev]

Attachment: nightly_pointer_events_ver6.diff

- Removed preferencies from b2g.js
- Removed unwanted ifdefs from firefox.js
- Removed preferencies from mobile.js
+ Added false preferencies to mulet.js

Suggestions and comments and objections are very welcome.

Comment 53

[Maksim Lebedev]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d3f9bd989aa8

Comment 54

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8622423 [details] [diff] [review]
nightly_pointer_events_ver6.diff

Review of attachment 8622423 [details] [diff] [review]:
-----------------------------------------------------------------

Assuming mulet.js overrides firefox.js (see bug 1174234) this should be fine.

Comment 55

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8622423 [details] [diff] [review]
nightly_pointer_events_ver6.diff

need to get answer to comment 54.

Comment 56

[Maksim Lebedev]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #54)
> Assuming mulet.js overrides firefox.js (see bug 1174234) this should be fine.
I downloaded build "Mulet Windows opt" with nightly_pointer_events_ver6.diff from Comment53
Looks like Pointer Events and touch-action are true by default, unfortunately.
So we can make proposition that mulet.js does not override firefox.js.
But what are the objections to turn on pointer events on Mulet?

Comment 57

[Maksim Lebedev]

Attachment: nightly_pointer_events_ver7.diff

+ Added explicity disabling into b2g.js

Suggestions and comments and objections are very welcome.

Comment 58

[Maksim Lebedev]

SEVEN version of patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a9ad12441591

Comment 59

[Maksim Lebedev]

I downloaded build "Mulet Windows opt" with nightly_pointer_events_ver7.diff from Comment58
Looks like Pointer Events and touch-actions are false by default, as expected.
So we can choose between SIX version of patch and SEVEN version of patch.
Olli Pettay, could You, please, put 'obsolet' flag to unwanted patch?

Comment 60

[Boris Zbarsky [:bzbarsky]]

> But what are the objections to turn on pointer events on Mulet?

Mulet claims to be "b2g" for test purposes.  The tests we have are checking that this stuff is not exposed on "b2g" because we don't want to expose it there (why, I dunno).  So if pointer events were exposed on Mulet, the tests would fail there.

Comment 61

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8623064 [details] [diff] [review]
nightly_pointer_events_ver7.diff

You don't need the mulet.js bits.

Past that, you need to disable on android, right?  Other than that, this is what I told you to do however many comments upthread....

Comment 62

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8623064 [details] [diff] [review]
nightly_pointer_events_ver7.diff

yeah, what bzbarsky said. With those changes this should be ok.

Comment 63

[Maksim Lebedev]

(In reply to Boris Zbarsky [:bz] from comment #61)
> You don't need the mulet.js bits.
According to opened bug 1174234 I assume that mulet.js will override b2g.js maybe.
And after that we can remove PE preferences from b2g.js
> Past that, you need to disable on android, right?  Other than that, this is
> what I told you to do however many comments upthread....
According to https://wiki.mozilla.org/Platform/Platform-specific_build_defines#Prefs_files I assume that android have all.js with disabled PE and possibly b2g.js with disabled PE. What are another changes I should make in this case?

Comment 64

[Boris Zbarsky [:bzbarsky]]

Oh, right.  Android doesn't load firefox.js.  So yeah, that should be ok.

Comment 65

[Maksim Lebedev]

Critical bug 1171101 and bug 1171712 were fixed. So we can push latest changes into m-c.
Or we should make any changes one more time?

Comment 66

[Maksim Lebedev]

If there are no objections, I put checkin-needed flag. We make a future... second attempt :-)

Comment 67

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3dcd15422d75

Comment 68

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3dcd15422d75

Comment 69

[Jean-Yves Perrier [:teoli]]

I've made a comment in https://developer.mozilla.org/en-US/docs/Web/CSS/pointer-events
and in Fx 41 for devs (making it clear that this doesn't read the train): https://developer.mozilla.org/en-US/Firefox/Releases/41#CSS

Comment 70

[Maksim Lebedev]

Just for clarification: CSS property "pointer-events" and "Pointer Events" which were enabled at current bug - are the different things. They have the same name - this is confusing. I hope that CSS property already works very long time at FireFox. But implementation of "Pointer Events" was enabled only several days ago. Feel free to see official specification: http://www.w3.org/TR/pointerevents

Comment 71

[Makoto Kato [:m_kato]]

Content editor should create the MDN PointerEvent page at https://developer.mozilla.org/en-US/docs/Web/API/PointerEvent

Comment 72

[Makoto Kato [:m_kato]]

moved PointerEvent from CSS to Web API

Also, for PointerEvent interface, create https://developer.mozilla.org/en-US/docs/Web/API/PointerEvent

Comment 73

[Jean-Yves Perrier [:teoli]]

Thanks, I mixed both things. I'm resetting dev-doc-needed.

Comment 74

[Alexey]

Maybe it's not related to this branch but this checkboxes: https://formstone.it/components/checkbox/ stopped working in FF41. It was OK in FF40 and is OK in other browsers (Chrome, Edge).
It works in FF41 only if I delete the first 3 lines from this piece of code:
if (Formstone.support.touch) {
  this.on( [Events.touchStart, Events.pointerDown].join(" "), data, onPointerStart);
} else 
{
  this.on(Events.click, data, onClick);
}

Comment 75

[Kartikaya Gupta (email:[e-mail])]

Alexey, please file a new bug for this issue if you believe it is a problem in Firefox. Thanks!

Comment 76

[Olli Pettay [:smaug][[e-mail]]]

I'm about to back this out until the implementation is more stable
(see bug 1181564).

https://treeherder.mozilla.org/#/jobs?repo=try&revision=dfc61793a9c7

Comment 77

[Olli Pettay [:smaug][[e-mail]]]

Attachment: backout patch

Comment 78

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4520f11055a7

Comment 79

[Ryan VanderMeulen [:RyanVM]]

Merge of backout:
https://hg.mozilla.org/mozilla-central/rev/4520f11055a7

Comment 80

[Hsin-Yi Tsai (she/her) [:hsinyi]]

I can't really tell the clear distinction between the scope of this bug and bug 822898. They both seem tracking the same thing at least speaking from now. I am going to close one and leave the other open for tracking the remaining work.

Comment 81

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Pointer Events shipped in 59 already. Should close this bug.

Comment 82

[Chris Mills [:cmills]]

Pointer Events docs were updated a while ago.