Enabling pointer events in Nightly builds

RESOLVED FIXED

Status

()

P3
normal
RESOLVED FIXED
4 years ago
5 days ago

People

(Reporter: alessarik, Assigned: alessarik)

Tracking

(Depends on: 3 bugs, Blocks: 2 bugs, {dev-doc-complete})

Trunk
dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 6 obsolete attachments)

(Assignee)

Description

4 years ago
Aim is enabling W3C pointer events in Nightly builds.
If anybody have objection about this action feel free to say into this bug.
(Assignee)

Comment 1

4 years ago
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.
Blocks: 960316
Depends on: 1122211
Flags: needinfo?(mbrubeck)
Flags: needinfo?(bugs)
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.
Flags: needinfo?(bugs)
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
Flags: needinfo?(mbrubeck)
(Assignee)

Comment 4

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

Suggestions and comments and objections are very welcome.
Assignee: nobody → alessarik
Status: NEW → ASSIGNED
Attachment #8609278 - Flags: review?(mbrubeck)
Attachment #8609278 - Flags: review?(bugs)
Attachment #8609278 - Flags: feedback?(jmathies)
Attachment #8609278 - Flags: feedback?(bugmail.mozilla)
Attachment #8609278 - Flags: feedback?(bugmail.mozilla) → feedback+
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+
Attachment #8609278 - Flags: review?(bugs) → review+
Comment on attachment 8609278 [details] [diff] [review]
nightly_pointer_events_ver1.diff

I had the same question as smaug in comment 6.
Attachment #8609278 - Flags: review?(mbrubeck)
(Assignee)

Comment 8

4 years ago
(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.
(Assignee)

Comment 9

4 years ago
+ Added positive preference for OSX.

Suggestions and comments and objections are very welcome.
Attachment #8609278 - Attachment is obsolete: true
Attachment #8609278 - Flags: feedback?(jmathies)
Attachment #8610014 - Flags: review?(mbrubeck)
Attachment #8610014 - Flags: review?(bugs)
Attachment #8610014 - Flags: feedback?(jmathies)
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.
Attachment #8610014 - Flags: review?(bugs) → review-
(Assignee)

Comment 12

4 years ago
(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))'
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)
(or add parentheses support to the preprocessor..)
(Assignee)

Comment 15

4 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #14)
> (or add parentheses support to the preprocessor..)
You have joked probably :-)
(Assignee)

Comment 16

4 years ago
+ Added obvious #if-else statements

Suggestions and comments and objections are very welcome.
Attachment #8610014 - Attachment is obsolete: true
Attachment #8610014 - Flags: review?(mbrubeck)
Attachment #8610014 - Flags: feedback?(jmathies)
Attachment #8610440 - Flags: review?(mbrubeck)
Attachment #8610440 - Flags: review?(bugs)
Attachment #8610440 - Flags: feedback?(jmathies)
Attachment #8610440 - Flags: feedback?(bugmail.mozilla)
(Assignee)

Updated

4 years ago
Blocks: 1133417, 1168319
Attachment #8610440 - Flags: feedback?(bugmail.mozilla)
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?
Attachment #8610440 - Flags: feedback?(jmathies) → feedback+
(Assignee)

Comment 19

4 years ago
(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 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.
Attachment #8610440 - Flags: review?(bugs) → review+
(Assignee)

Comment 21

4 years ago
(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 :-)
Attachment #8610440 - Flags: review?(mbrubeck) → review+
(Assignee)

Comment 22

4 years ago
If there are no objections, I will put checkin-needed flag...
...at the same time as bug 1133417 and bug 1168319.
(Assignee)

Comment 23

4 years ago
If there are no objections, I put checkin-needed flag. We make a future...
Keywords: checkin-needed
OS: Unspecified → All
Hardware: Unspecified → All
does this last patches fix issues from the try run like https://treeherder.mozilla.org/logviewer.html#?job_id=7886131&repo=try ?
Flags: needinfo?(alessarik)
Keywords: checkin-needed
(Assignee)

Comment 25

4 years ago
(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.
Flags: needinfo?(alessarik)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5458f49a6f85
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox41: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
(Assignee)

Updated

4 years ago
Blocks: 1171015
Blocks: 1170792
No longer blocks: 1171015
Depends on: 1171015
Depends on: 1171101
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).
yes, we should back this out asap.
Backed out in https://hg.mozilla.org/mozilla-central/rev/a8edd0679b57 for the scrolling fail.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 31

4 years ago
+ PointerEvents disable:true  ->  nightly:true & desktop:true

Suggestions and comments and objections are very welcome.
Attachment #8615281 - Flags: review?(bugs)
Attachment #8615281 - Flags: feedback?(bzbarsky)
(Assignee)

Comment 32

4 years ago
+ Added explicity disabling pointer event for Android and B2G platforms


Suggestions and comments and objections are very welcome.
Attachment #8610440 - Attachment is obsolete: true
Attachment #8615283 - Flags: review?(bugs)
Attachment #8615283 - Flags: feedback?(mbrubeck)
Attachment #8615283 - Flags: feedback?(jmathies)
Attachment #8615283 - Flags: feedback?(bzbarsky)
Attachment #8615283 - Flags: feedback?(bugmail.mozilla)
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.
Attachment #8615283 - Flags: feedback?(bugmail.mozilla)
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 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.
Attachment #8615283 - Flags: review?(bugs)
Attachment #8615283 - Flags: feedback?(mbrubeck)
Attachment #8615283 - Flags: feedback?(jmathies)
Attachment #8615283 - Flags: feedback?(bzbarsky)
Attachment #8615281 - Flags: review?(bugs) → review+
Bug 1171101 must be fixed before this bug.
or, in other words, when fixing this again, that bug must not regress.
Duplicate of this bug: 1171187
Keywords: dev-doc-needed
This breaks Linux as well, so the maybe Linux only idea will not fly.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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.
> 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.
Attachment #8615281 - Flags: feedback?(bzbarsky) → feedback+
Duplicate of this bug: 1171022
(Assignee)

Updated

4 years ago
Depends on: 1171712
(Assignee)

Updated

4 years ago
Depends on: 1171151
(Assignee)

Comment 44

4 years ago
+ 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.
Attachment #8615283 - Attachment is obsolete: true
Attachment #8621599 - Flags: review?(bugs)
Attachment #8621599 - Flags: feedback?(mbrubeck)
Attachment #8621599 - Flags: feedback?(jmathies)
Attachment #8621599 - Flags: feedback?(bzbarsky)
Attachment #8621599 - Flags: feedback?(bugmail.mozilla)
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.
Attachment #8621599 - Flags: feedback?(bugmail.mozilla)
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.
Attachment #8621599 - Flags: feedback?(bzbarsky) → feedback+
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.
Attachment #8621599 - Flags: review?(bugs) → review+
And since the prefs are false by default, just have the prefs enabled in firefox.js
Attachment #8621599 - Flags: review+ → review-
Attachment #8621599 - Flags: feedback?(mbrubeck)
> 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.
Oh, right. That is silly.
(Assignee)

Comment 52

4 years ago
- 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.
Attachment #8621599 - Attachment is obsolete: true
Attachment #8621599 - Flags: feedback?(jmathies)
Attachment #8622423 - Flags: review?(bugs)
Attachment #8622423 - Flags: feedback?(mbrubeck)
Attachment #8622423 - Flags: feedback?(jmathies)
Attachment #8622423 - Flags: feedback?(bzbarsky)
Attachment #8622423 - Flags: feedback?(bugmail.mozilla)
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.
Attachment #8622423 - Flags: feedback?(bugmail.mozilla) → feedback+

Updated

4 years ago
Attachment #8622423 - Flags: feedback?(jmathies)
Attachment #8622423 - Flags: feedback?(mbrubeck) → feedback+
Attachment #8622423 - Flags: feedback?(bzbarsky) → feedback+
Comment on attachment 8622423 [details] [diff] [review]
nightly_pointer_events_ver6.diff

need to get answer to comment 54.
Attachment #8622423 - Flags: review?(bugs)
(Assignee)

Comment 56

4 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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?
Flags: needinfo?(bugs)
(Assignee)

Comment 57

4 years ago
+ Added explicity disabling into b2g.js

Suggestions and comments and objections are very welcome.
Attachment #8623064 - Flags: review?(bugs)
Attachment #8623064 - Flags: feedback?(mbrubeck)
Attachment #8623064 - Flags: feedback?(bzbarsky)
Attachment #8623064 - Flags: feedback?(bugmail.mozilla)
(Assignee)

Comment 59

4 years ago
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?
(Assignee)

Updated

4 years ago
Attachment #8622423 - Flags: review?(bugs)
> 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 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....
Attachment #8623064 - Flags: feedback?(bzbarsky) → feedback+
Comment on attachment 8623064 [details] [diff] [review]
nightly_pointer_events_ver7.diff

yeah, what bzbarsky said. With those changes this should be ok.
Flags: needinfo?(bugs)
Attachment #8623064 - Flags: review?(bugs) → review+
Attachment #8622423 - Attachment is obsolete: true
Attachment #8622423 - Flags: review?(bugs)
Attachment #8623064 - Flags: feedback?(bugmail.mozilla)
(Assignee)

Comment 63

4 years ago
(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?
Flags: needinfo?(bzbarsky)
Oh, right.  Android doesn't load firefox.js.  So yeah, that should be ok.
Flags: needinfo?(bzbarsky)
(Assignee)

Comment 65

4 years ago
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?
Flags: needinfo?(bugs)
Attachment #8623064 - Flags: feedback?(mbrubeck) → feedback+
(Assignee)

Comment 66

4 years ago
If there are no objections, I put checkin-needed flag. We make a future... second attempt :-)
Keywords: checkin-needed
Flags: needinfo?(bugs)
https://hg.mozilla.org/mozilla-central/rev/3dcd15422d75
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
Component: Untriaged → DOM
Product: Firefox → Core
Target Milestone: Firefox 41 → mozilla41
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
Keywords: dev-doc-needed → dev-doc-complete
(Assignee)

Comment 70

4 years ago
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
Flags: needinfo?(jypenator)
Content editor should create the MDN PointerEvent page at https://developer.mozilla.org/en-US/docs/Web/API/PointerEvent
moved PointerEvent from CSS to Web API

Also, for PointerEvent interface, create https://developer.mozilla.org/en-US/docs/Web/API/PointerEvent
Thanks, I mixed both things. I'm resetting dev-doc-needed.
Flags: needinfo?(jypenator)
Keywords: dev-doc-complete → dev-doc-needed

Updated

4 years ago
Depends on: 1181564

Updated

4 years ago
Summary: Enabling pointer events in Nighly builds → Enabling pointer events in Nightly builds

Comment 74

4 years ago
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);
}
Alexey, please file a new bug for this issue if you believe it is a problem in Firefox. Thanks!
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Merge of backout:
https://hg.mozilla.org/mozilla-central/rev/4520f11055a7
status-firefox41: fixed → ---
Target Milestone: mozilla41 → ---
Depends on: 1258804
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.
No longer depends on: 1031362
Depends on: 1315189, 1352278
Assignee: alessarik → nobody

Updated

9 months ago
Priority: -- → P3
Blocks: 1487509
Pointer Events shipped in 59 already. Should close this bug.
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 months ago
Resolution: --- → FIXED
No longer depends on: 1315189
Assignee: nobody → alessarik
Pointer Events docs were updated a while ago.
Keywords: dev-doc-needed → dev-doc-complete
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.