New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 675549 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Launch-OWP
Launch-Accessibility: ----
Launch-Exp-Leadership: ----
Launch-Leadership: ----
Launch-Legal: ----
Launch-M-Approved: ----
Launch-M-Target: ----
Launch-Privacy: ----
Launch-Security: ----
Launch-Test: ----
Launch-UI: ----
Rollout-Type: ----



Sign in to add a comment

Make setting Event's cancelBubble to false a no-op

Project Member Reported by annevank...@gmail.com, Dec 19 2016

Issue description

Comment 1 by tkent@chromium.org, Dec 19 2016

Components: Blink>DOM>Events
Labels: Needs-BlinkIntent
Status: Available (was: Untriaged)

Comment 2 by rbyers@chromium.org, Dec 21 2016

Cc: dtapu...@chromium.org foolip@chromium.org
Labels: -Type-Bug OWP-Type-Deprecation Hotlist-GoodFirstBug Type-Launch-OWP
Given the compat analysis already done at https://github.com/whatwg/dom/issues/211, I suspect this will be really easy (but does still need an intent - http://www.chromium.org/blink#TOC-Policy-for-shipping-and-removing-web-platform-API-features).

Given the obscurity I probably wouldn't bother with any outreach (no chromestatus entry), but we should probably still have a one milestone deprecation warning period just in case.  

Making the EventCancelBubbleWasChangedToFalse counter into a Deprecation will be trivial: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/events/Event.cpp?sq=package:chromium&dr=CSs&rcl=1482265342&l=343.  Then a milestone later we should probably replace that counter with a simple console warning saying the method is a no-op and early out.

This is probably a good chance for someone to do their first blink "intent to deprecate and remove" if someone is interested.

Comment 3 by l...@chromium.org, Jan 18 2017

Owner: l...@chromium.org
Status: Assigned (was: Available)
I am interested in doing it, assign to myself.

Feature freeze of M57 is Jan 06 2017, does that mean in "Intent to deprecate" we should indicate we are going to deprecate it in M58?

Comment 4 by foolip@chromium.org, Jan 18 2017

If you send out an Intent to Deprecate and Remove, deprecating in M58 and making the change in M59 (or later, could make it M61) makes sense. Not so much because of the feature freeze date but just because there might not be time to get 3xLGTM before the branch point. (Merging back to M57 is possible but if there's no hurry not necessary.)

Comment 5 Deleted

Comment 6 by cvrebert@google.com, Jan 20 2017

Issue 657651 has been merged into this issue.
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b461901d4922144f8cd88810a60e23a3a5d08e4c

commit b461901d4922144f8cd88810a60e23a3a5d08e4c
Author: lpy <lpy@chromium.org>
Date: Wed Jan 25 21:13:00 2017

Remove setting cancelBubble to false.

According to spec https://dom.spec.whatwg.org/#dom-event-cancelbubble, setting
cancelBubble to true is considered as an alias to stopPropagation(), setting
cancelBubble to false is a no-op. Detail discussion is on
https://github.com/whatwg/dom/issues/211.

BUG= 675549 

Review-Url: https://codereview.chromium.org/2641953006
Cr-Commit-Position: refs/heads/master@{#446118}

[delete] https://crrev.com/76a10b17535c0456f042817a69fa036f226b26a3/third_party/WebKit/LayoutTests/external/wpt/dom/events/Event-cancelBubble-expected.txt
[delete] https://crrev.com/76a10b17535c0456f042817a69fa036f226b26a3/third_party/WebKit/LayoutTests/external/wpt/dom/events/Event-dispatch-bubble-canceled-expected.txt
[delete] https://crrev.com/76a10b17535c0456f042817a69fa036f226b26a3/third_party/WebKit/LayoutTests/external/wpt/dom/events/Event-dispatch-multiple-cancelBubble-expected.txt
[delete] https://crrev.com/76a10b17535c0456f042817a69fa036f226b26a3/third_party/WebKit/LayoutTests/external/wpt/dom/events/Event-propagation-expected.txt
[modify] https://crrev.com/b461901d4922144f8cd88810a60e23a3a5d08e4c/third_party/WebKit/Source/core/events/Event.cpp
[modify] https://crrev.com/b461901d4922144f8cd88810a60e23a3a5d08e4c/third_party/WebKit/Source/core/events/Event.h
[modify] https://crrev.com/b461901d4922144f8cd88810a60e23a3a5d08e4c/third_party/WebKit/Source/core/events/EventDispatcher.cpp
[modify] https://crrev.com/b461901d4922144f8cd88810a60e23a3a5d08e4c/third_party/WebKit/Source/core/frame/UseCounter.h
[modify] https://crrev.com/b461901d4922144f8cd88810a60e23a3a5d08e4c/third_party/WebKit/Source/modules/indexeddb/IDBEventDispatcher.cpp
[modify] https://crrev.com/b461901d4922144f8cd88810a60e23a3a5d08e4c/tools/metrics/histograms/histograms.xml

Comment 8 by l...@chromium.org, Feb 6 2017

Shall I just close this bug as fixed?
Cc: jmedley@chromium.org
Labels: M-58
I think so. I added the M-58 label, do you look at any other information, jmedley@?
I've got it on my list. Do you have stats on how many pages this could break? I'm not convinced it can do without a Chrome Status entry.
The relevant counter was https://www.chromestatus.com/metrics/feature/timeline/popularity/1350, so not exactly zero but rounding to zero.

Comment 12 by tkent@chromium.org, Feb 16 2017

Labels: -Needs-BlinkIntent
Status: Fixed (was: Assigned)

Comment 13 by tkent@chromium.org, Mar 15 2017

Components: -Blink>DOM>Events Blink>DOM
Remove Blink>DOM>Events

Sign in to add a comment