|
|
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} |