Make textContent trigger single DOMSubtreeModified event

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

[rwlbuis, 2014-10-31 17:35:50]

rob.buis@samsung.com changed reviewers:
+ sigbjorn@opera.com

[rwlbuis, 2014-10-31 17:35:50]

PTAL

[rwlbuis, 2014-11-19 22:11:34]

On 2014/10/31 17:35:50, rwlbuis wrote:
> PTAL

ping?

[kouhei (in TOK), 2014-11-19 23:26:19]

kouhei@chromium.org changed reviewers:
+ kouhei@chromium.org, sigbjornf@opera.com

[kouhei (in TOK), 2014-11-19 23:26:20]

+sigbjornf

[kouhei (in TOK), 2014-11-19 23:28:54]

https://codereview.chromium.org/688933002/diff/40001/Source/core/dom/Containe...
File Source/core/dom/ContainerNode.h (right):

https://codereview.chromium.org/688933002/diff/40001/Source/core/dom/Containe...
Source/core/dom/ContainerNode.h:102: void removeChildren(bool sendModifiedEvent
= true);
Please consider using an enum here.

[sof, 2014-11-20 12:10:00]

https://codereview.chromium.org/688933002/diff/40001/Source/core/dom/Containe...
File Source/core/dom/ContainerNode.cpp (right):

https://codereview.chromium.org/688933002/diff/40001/Source/core/dom/Containe...
Source/core/dom/ContainerNode.cpp:674: // We don't fire the DOMSubtreeModified
event for Attr Nodes. This matches the behavior
Move this comment to Attr::setValue()

https://codereview.chromium.org/688933002/diff/40001/Source/core/dom/Containe...
File Source/core/dom/ContainerNode.h (right):

https://codereview.chromium.org/688933002/diff/40001/Source/core/dom/Containe...
Source/core/dom/ContainerNode.h:102: void removeChildren(bool sendModifiedEvent
= true);
On 2014/11/19 23:28:54, kouhei wrote:
> Please consider using an enum here.

Agree; in this case, there's no but-of-course interpretation of a boolean
argument either.

https://codereview.chromium.org/688933002/diff/40001/Source/core/html/HTMLTit...
File Source/core/html/HTMLTitleElement.cpp (right):

https://codereview.chromium.org/688933002/diff/40001/Source/core/html/HTMLTit...
Source/core/html/HTMLTitleElement.cpp:88: removeChildren(false);
Would be good to have test coverage for this.

[rwlbuis, 2014-11-20 18:20:52]

On 2014/11/20 12:10:00, sof wrote:
>
https://codereview.chromium.org/688933002/diff/40001/Source/core/dom/Containe...
> File Source/core/dom/ContainerNode.cpp (right):
> 
>
https://codereview.chromium.org/688933002/diff/40001/Source/core/dom/Containe...
> Source/core/dom/ContainerNode.cpp:674: // We don't fire the DOMSubtreeModified
> event for Attr Nodes. This matches the behavior
> Move this comment to Attr::setValue()

Done.

>
https://codereview.chromium.org/688933002/diff/40001/Source/core/dom/Containe...
> File Source/core/dom/ContainerNode.h (right):
> 
>
https://codereview.chromium.org/688933002/diff/40001/Source/core/dom/Containe...
> Source/core/dom/ContainerNode.h:102: void removeChildren(bool
sendModifiedEvent
> = true);
> On 2014/11/19 23:28:54, kouhei wrote:
> > Please consider using an enum here.
> 
> Agree; in this case, there's no but-of-course interpretation of a boolean
> argument either.

The enum does seem to clean it up a bit. I put it in global namespace to ease
using it in Node.cpp.

>
https://codereview.chromium.org/688933002/diff/40001/Source/core/html/HTMLTit...
> File Source/core/html/HTMLTitleElement.cpp (right):
> 
>
https://codereview.chromium.org/688933002/diff/40001/Source/core/html/HTMLTit...
> Source/core/html/HTMLTitleElement.cpp:88: removeChildren(false);
> Would be good to have test coverage for this.

I added a test.

[sof, 2014-11-20 19:51:41]

lgtm

Could you extend the description to include some compat details for
setTextContent() and this event? i.e., aligning with Gecko/FF.

[rwlbuis, 2014-11-20 21:13:14]

The CQ bit was checked by rob.buis@samsung.com

[commit-bot: I haz the power, 2014-11-20 21:15:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/688933002/60001

[commit-bot: I haz the power, 2014-11-20 21:32:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-20 21:32:24]

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)

[rwlbuis, 2014-11-20 21:40:23]

The CQ bit was checked by rob.buis@samsung.com

[commit-bot: I haz the power, 2014-11-20 21:40:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/688933002/60001

[commit-bot: I haz the power, 2014-11-20 22:52:27]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=185712