Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(369)

Issue 688933002: Make textContent trigger single DOMSubtreeModified event (Closed)

Created:
6 years, 1 month ago by rwlbuis
Modified:
6 years, 1 month ago
CC:
blink-reviews, ed+blinkwatch_opera.com, blink-reviews-html_chromium.org, sof, eae+blinkwatch, fs, kouhei+svg_chromium.org, blink-reviews-dom_chromium.org, dglazkov+blink, krit, f(malita), gyuyoung.kim_webkit.org, Stephen Chennney, pdr+svgwatchlist_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Make textContent trigger single DOMSubtreeModified event In order to control whether removeChildren emits a DOMSubtreeModifiedEvent add an enum parameter to it. Make attribute never emit it, as well as some other sites where we do an internal removeChildren that should not be exposed. For setTextContent change the logic so always a single DOMSubtreeModified event is emitted. The new behavior for textContent DOMSubtreeModified triggering matches Firefox. BUG=332070 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185712

Patch Set 1 #

Patch Set 2 : Fix test #

Patch Set 3 : Add test #

Total comments: 4

Patch Set 4 : Address comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+50 lines, -13 lines) Patch
A LayoutTests/fast/dom/Node/textContent-single-DOMSubtreeModified.html View 1 chunk +11 lines, -0 lines 0 comments Download
A LayoutTests/fast/dom/Node/textContent-single-DOMSubtreeModified-expected.html View 1 chunk +2 lines, -0 lines 0 comments Download
A LayoutTests/fast/dom/document-set-title-single-DOMSubtreeModified.html View 1 chunk +15 lines, -0 lines 0 comments Download
A + LayoutTests/fast/dom/document-set-title-single-DOMSubtreeModified-expected.txt View 1 chunk +3 lines, -2 lines 0 comments Download
M Source/core/dom/Attr.cpp View 1 chunk +4 lines, -1 line 0 comments Download
Source/core/dom/ContainerNode.h View 2 chunks +6 lines, -1 line 0 comments Download
M Source/core/dom/ContainerNode.cpp View 2 chunks +2 lines, -5 lines 0 comments Download
M Source/core/dom/Node.cpp View 1 chunk +5 lines, -2 lines 0 comments Download
M Source/core/html/HTMLTitleElement.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/svg/SVGUseElement.cpp View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 16 (5 generated)
rwlbuis
PTAL
6 years, 1 month ago (2014-10-31 17:35:50 UTC) #2
rwlbuis
On 2014/10/31 17:35:50, rwlbuis wrote: > PTAL ping?
6 years, 1 month ago (2014-11-19 22:11:34 UTC) #3
kouhei (in TOK)
+sigbjornf
6 years, 1 month ago (2014-11-19 23:26:20 UTC) #5
kouhei (in TOK)
https://codereview.chromium.org/688933002/diff/40001/Source/core/dom/ContainerNode.h File Source/core/dom/ContainerNode.h (right): https://codereview.chromium.org/688933002/diff/40001/Source/core/dom/ContainerNode.h#newcode102 Source/core/dom/ContainerNode.h:102: void removeChildren(bool sendModifiedEvent = true); Please consider using an ...
6 years, 1 month ago (2014-11-19 23:28:54 UTC) #6
sof
https://codereview.chromium.org/688933002/diff/40001/Source/core/dom/ContainerNode.cpp File Source/core/dom/ContainerNode.cpp (right): https://codereview.chromium.org/688933002/diff/40001/Source/core/dom/ContainerNode.cpp#newcode674 Source/core/dom/ContainerNode.cpp:674: // We don't fire the DOMSubtreeModified event for Attr ...
6 years, 1 month ago (2014-11-20 12:10:00 UTC) #7
rwlbuis
On 2014/11/20 12:10:00, sof wrote: > https://codereview.chromium.org/688933002/diff/40001/Source/core/dom/ContainerNode.cpp > File Source/core/dom/ContainerNode.cpp (right): > > https://codereview.chromium.org/688933002/diff/40001/Source/core/dom/ContainerNode.cpp#newcode674 > ...
6 years, 1 month ago (2014-11-20 18:20:52 UTC) #8
sof
lgtm Could you extend the description to include some compat details for setTextContent() and this ...
6 years, 1 month ago (2014-11-20 19:51:41 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/688933002/60001
6 years, 1 month ago (2014-11-20 21:15:46 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: mac_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/31958)
6 years, 1 month ago (2014-11-20 21:32:24 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/688933002/60001
6 years, 1 month ago (2014-11-20 21:40:57 UTC) #15
commit-bot: I haz the power
6 years, 1 month ago (2014-11-20 22:52:27 UTC) #16
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=185712

Powered by Google App Engine
This is Rietveld 408576698