Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2375493005:
Enabled Pointer Events API in stable. (Closed)
DescriptionEnabled Pointer Events API in stable.
Intent to ship: https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/UY1EfjaGF_E/OU7W_s3PAgAJ
BUG=196799
Committed: https://crrev.com/902a3d6303dfbfd49a24173777afcb4b70234a17
Cr-Commit-Position: refs/heads/master@{#421811}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Matched content_features.cc state. #Patch Set 3 : Fixed runtime_features.cc #
Messages
Total messages: 27 (12 generated)
The CQ bit was checked by mustaq@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
mustaq@chromium.org changed reviewers: + dtapuska@chromium.org, rbyers@chromium.org
ptal
https://codereview.chromium.org/2375493005/diff/1/third_party/WebKit/Source/p... File third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in (right): https://codereview.chromium.org/2375493005/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in:170: PointerEvent status=stable I'd like https://cs.chromium.org/chromium/src/content/public/common/content_features.c... adjusted to match Please also check that --disable-features=PointerEvent works correctly.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
Please add a link to the intent to ship thread in the CL description. LGTM (pending Dave's feedback of course).
Please add a link to the intent to ship thread in the CL description. LGTM (pending Dave's feedback of course).
Seems with PointerEvent=stable, the feature is active no matter what the command-line (--disable-features=PointerEvent) or UI flags (chrome://flags/#enable-pointer-events) says! https://codereview.chromium.org/2375493005/diff/1/third_party/WebKit/Source/p... File third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in (right): https://codereview.chromium.org/2375493005/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in:170: PointerEvent status=stable On 2016/09/27 18:38:27, dtapuska wrote: > I'd like > https://cs.chromium.org/chromium/src/content/public/common/content_features.c... > adjusted to match > > Please also check that --disable-features=PointerEvent works correctly. Done.
On 2016/09/28 15:58:12, mustaq wrote: > Seems with PointerEvent=stable, the feature is active no matter what the > command-line (--disable-features=PointerEvent) or UI flags > (chrome://flags/#enable-pointer-events) says! Thanks for checking. That seems important to fix (for users struggling with a site with bugs that make it unusable when pointer events are enabled), but could be fixed in a follow-up CL. Worst case you could add a new flag for force-disable, but ideally there's just some way to fix the feature plumbing. > https://codereview.chromium.org/2375493005/diff/1/third_party/WebKit/Source/p... > File third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in (right): > > https://codereview.chromium.org/2375493005/diff/1/third_party/WebKit/Source/p... > third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in:170: PointerEvent > status=stable > On 2016/09/27 18:38:27, dtapuska wrote: > > I'd like > > > https://cs.chromium.org/chromium/src/content/public/common/content_features.c... > > adjusted to match > > > > Please also check that --disable-features=PointerEvent works correctly. > > Done.
On 2016/09/28 16:00:56, Rick Byers wrote:
> On 2016/09/28 15:58:12, mustaq wrote:
> > Seems with PointerEvent=stable, the feature is active no matter what the
> > command-line (--disable-features=PointerEvent) or UI flags
> > (chrome://flags/#enable-pointer-events) says!
>
> Thanks for checking. That seems important to fix (for users struggling with a
> site with bugs that make it unusable when pointer events are enabled), but
could
> be fixed in a follow-up CL. Worst case you could add a new flag for
> force-disable, but ideally there's just some way to fix the feature plumbing.
>
Confusingly, "--disable-blink-features=PointerEvent" works!
Dave: wondering how the Finch config controls the feature state? Is it through
{enable,disable}-blink-features or {enable,disable}-features? We should avoid
bumpy UseCount history for our field trials I believe, even for Canary.
On 2016/09/28 16:22:51, mustaq wrote:
> On 2016/09/28 16:00:56, Rick Byers wrote:
> > On 2016/09/28 15:58:12, mustaq wrote:
> > > Seems with PointerEvent=stable, the feature is active no matter what the
> > > command-line (--disable-features=PointerEvent) or UI flags
> > > (chrome://flags/#enable-pointer-events) says!
> >
> > Thanks for checking. That seems important to fix (for users struggling with
a
> > site with bugs that make it unusable when pointer events are enabled), but
> could
> > be fixed in a follow-up CL. Worst case you could add a new flag for
> > force-disable, but ideally there's just some way to fix the feature
plumbing.
> >
>
> Confusingly, "--disable-blink-features=PointerEvent" works!
>
> Dave: wondering how the Finch config controls the feature state? Is it through
> {enable,disable}-blink-features or {enable,disable}-features? We should avoid
> bumpy UseCount history for our field trials I believe, even for Canary.
Please change
https://cs.chromium.org/chromium/src/content/child/runtime_features.cc?sq=pac...
to
WebRuntimeFeatures::enablePointerEvent(base::FeatureList::IsEnabled(features::kPointerEvents));
Description was changed from ========== Enabled Pointer Events API in stable. BUG=196799 ========== to ========== Enabled Pointer Events API in stable. Intent to ship: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/UY1EfjaGF_E BUG=196799 ==========
Description was changed from ========== Enabled Pointer Events API in stable. Intent to ship: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/UY1EfjaGF_E BUG=196799 ========== to ========== Enabled Pointer Events API in stable. Intent to ship: https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/UY1EfjaGF_E/OU... BUG=196799 ==========
On 2016/09/28 16:40:31, dtapuska wrote:
> On 2016/09/28 16:22:51, mustaq wrote:
> > On 2016/09/28 16:00:56, Rick Byers wrote:
> > > On 2016/09/28 15:58:12, mustaq wrote:
> > > > Seems with PointerEvent=stable, the feature is active no matter what the
> > > > command-line (--disable-features=PointerEvent) or UI flags
> > > > (chrome://flags/#enable-pointer-events) says!
> > >
> > > Thanks for checking. That seems important to fix (for users struggling
with
> a
> > > site with bugs that make it unusable when pointer events are enabled), but
> > could
> > > be fixed in a follow-up CL. Worst case you could add a new flag for
> > > force-disable, but ideally there's just some way to fix the feature
> plumbing.
> > >
> >
> > Confusingly, "--disable-blink-features=PointerEvent" works!
> >
> > Dave: wondering how the Finch config controls the feature state? Is it
through
> > {enable,disable}-blink-features or {enable,disable}-features? We should
avoid
> > bumpy UseCount history for our field trials I believe, even for Canary.
>
> Please change
>
https://cs.chromium.org/chromium/src/content/child/runtime_features.cc?sq=pac...
> to
>
>
WebRuntimeFeatures::enablePointerEvent(base::FeatureList::IsEnabled(features::kPointerEvents));
Done, confirmed that it works.
On 2016/09/28 17:35:27, mustaq wrote:
> On 2016/09/28 16:40:31, dtapuska wrote:
> > On 2016/09/28 16:22:51, mustaq wrote:
> > > On 2016/09/28 16:00:56, Rick Byers wrote:
> > > > On 2016/09/28 15:58:12, mustaq wrote:
> > > > > Seems with PointerEvent=stable, the feature is active no matter what
the
> > > > > command-line (--disable-features=PointerEvent) or UI flags
> > > > > (chrome://flags/#enable-pointer-events) says!
> > > >
> > > > Thanks for checking. That seems important to fix (for users struggling
> with
> > a
> > > > site with bugs that make it unusable when pointer events are enabled),
but
> > > could
> > > > be fixed in a follow-up CL. Worst case you could add a new flag for
> > > > force-disable, but ideally there's just some way to fix the feature
> > plumbing.
> > > >
> > >
> > > Confusingly, "--disable-blink-features=PointerEvent" works!
> > >
> > > Dave: wondering how the Finch config controls the feature state? Is it
> through
> > > {enable,disable}-blink-features or {enable,disable}-features? We should
> avoid
> > > bumpy UseCount history for our field trials I believe, even for Canary.
> >
> > Please change
> >
>
https://cs.chromium.org/chromium/src/content/child/runtime_features.cc?sq=pac...
> > to
> >
> >
>
WebRuntimeFeatures::enablePointerEvent(base::FeatureList::IsEnabled(features::kPointerEvents));
>
> Done, confirmed that it works.
lgtm
mustaq@chromium.org changed reviewers: + jochen@chromium.org
jochen@chromium.org: Please review changes in content/
lgtm
The CQ bit was checked by dtapuska@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rbyers@chromium.org Link to the patchset: https://codereview.chromium.org/2375493005/#ps40001 (title: "Fixed runtime_features.cc")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Enabled Pointer Events API in stable. Intent to ship: https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/UY1EfjaGF_E/OU... BUG=196799 ========== to ========== Enabled Pointer Events API in stable. Intent to ship: https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/UY1EfjaGF_E/OU... BUG=196799 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Enabled Pointer Events API in stable. Intent to ship: https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/UY1EfjaGF_E/OU... BUG=196799 ========== to ========== Enabled Pointer Events API in stable. Intent to ship: https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/UY1EfjaGF_E/OU... BUG=196799 Committed: https://crrev.com/902a3d6303dfbfd49a24173777afcb4b70234a17 Cr-Commit-Position: refs/heads/master@{#421811} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/902a3d6303dfbfd49a24173777afcb4b70234a17 Cr-Commit-Position: refs/heads/master@{#421811} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
