Setting empty script text should remove child node of script element

Setting empty script text should remove child node of script element

BUG=520849
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200816

[ramya.v, 2015-08-18 13:37:54]

ramya.v@samsung.com changed reviewers:
+ tkent@chromium.org

[ramya.v, 2015-08-18 13:37:55]

PTAL!

Thanks

[tkent, 2015-08-19 00:11:26]

Please update LayoutTests/TestExpectations.

https://codereview.chromium.org/1292503003/diff/1/Source/core/html/HTMLScript...
File Source/core/html/HTMLScriptElement.cpp (right):

https://codereview.chromium.org/1292503003/diff/1/Source/core/html/HTMLScript...
Source/core/html/HTMLScriptElement.cpp:131: RefPtrWillBeRawPtr<Node>
protectFromMutationEvents(this);
Can we just call setTextContent(value), and add isEmpty() check to
Node::setTextContent()?

[ramya.v, 2015-08-19 08:08:01]

On 2015/08/19 00:11:26, tkent wrote:
> Please update LayoutTests/TestExpectations.
> 
>
https://codereview.chromium.org/1292503003/diff/1/Source/core/html/HTMLScript...
> File Source/core/html/HTMLScriptElement.cpp (right):
> 
>
https://codereview.chromium.org/1292503003/diff/1/Source/core/html/HTMLScript...
> Source/core/html/HTMLScriptElement.cpp:131: RefPtrWillBeRawPtr<Node>
> protectFromMutationEvents(this);
> Can we just call setTextContent(value), and add isEmpty() check to
> Node::setTextContent()?

Updated as per review comments.
PTAL!


Thanks

[ramya.v, 2015-08-19 08:08:42]

Updated as per review comments. PTAL!

Thanks

https://codereview.chromium.org/1292503003/diff/1/Source/core/html/HTMLScript...
File Source/core/html/HTMLScriptElement.cpp (right):

https://codereview.chromium.org/1292503003/diff/1/Source/core/html/HTMLScript...
Source/core/html/HTMLScriptElement.cpp:131: RefPtrWillBeRawPtr<Node>
protectFromMutationEvents(this);
On 2015/08/19 00:11:26, tkent wrote:
> Can we just call setTextContent(value), and add isEmpty() check to
> Node::setTextContent()?

Modified code to call setTextContent. isEmpty() check is already present in
Node::setTextContent.
Done.

[tkent, 2015-08-19 08:11:21]

lgtm with a nit

https://codereview.chromium.org/1292503003/diff/20001/Source/core/html/HTMLSc...
File Source/core/html/HTMLScriptElement.cpp (right):

https://codereview.chromium.org/1292503003/diff/20001/Source/core/html/HTMLSc...
Source/core/html/HTMLScriptElement.cpp:131: RefPtrWillBeRawPtr<Node>
protectFromMutationEvents(this);
This line is unnecessary.  setTextContent has similar protection object
|container|.

[ramya.v, 2015-08-19 08:14:18]

Updated as per review comments.

https://codereview.chromium.org/1292503003/diff/20001/Source/core/html/HTMLSc...
File Source/core/html/HTMLScriptElement.cpp (right):

https://codereview.chromium.org/1292503003/diff/20001/Source/core/html/HTMLSc...
Source/core/html/HTMLScriptElement.cpp:131: RefPtrWillBeRawPtr<Node>
protectFromMutationEvents(this);
On 2015/08/19 08:11:21, tkent wrote:
> This line is unnecessary.  setTextContent has similar protection object
> |container|.

Done.

[ramya.v, 2015-08-19 08:17:45]

The CQ bit was checked by ramya.v@samsung.com

[ramya.v, 2015-08-19 08:17:46]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org
Link to the patchset: https://codereview.chromium.org/1292503003/#ps40001
(title: "Updated as per review comments")

[commit-bot: I haz the power, 2015-08-19 08:17:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1292503003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1292503003/40001

[commit-bot: I haz the power, 2015-08-19 11:29:08]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=200816