Bug 60534 – Crashes if the document inside iframe is removed during pasting some text into it.

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Hajime Morrita]

Repro and patch will some shortly.

[Hajime Morrita]

Created attachment 92929 [details]
Patch

[Kent Tamura]

Comment on attachment 92929 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=92929&action=review

> LayoutTests/editing/pasteboard/resources/paste-removing-iframe-child.html:12
> +        // Calls notifyDone() before because the removal
> +        // can terminate the scdript execution.
> +        if (window.parent.layoutTestController)
> +            window.parent.layoutTestController.notifyDone();
> +        var toRemove = window.parent.document.getElementById("child");
> +        toRemove.parentNode.removeChild(toRemove);

Does this correctly crash without the Editor.cpp change?
I think notifyDone() immediately terminates the test.

[Hajime Morrita]

Hi Kent-san, thank you for taking a look!

> Does this correctly crash without the Editor.cpp change?
> I think notifyDone() immediately terminates the test.
It works for Mac DRT which just set flag on notifyDone() to exit the event loop.
But I have no idea for other port.
So I'd like to search safer way.

[Kent Tamura]

(In reply to comment #3)
I think we can use a DOM mutation event in the parent document.

[Ryosuke Niwa]

Comment on attachment 92929 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=92929&action=review

> LayoutTests/editing/pasteboard/resources/paste-removing-iframe-child.html:8
> +        // can terminate the scdript execution.

Typo: scdript

> LayoutTests/editing/pasteboard/resources/paste-removing-iframe-child.html:10
> +        if (window.parent.layoutTestController)
> +            window.parent.layoutTestController.notifyDone();

You can't call notifyDone before removing the node.

[Ryosuke Niwa]

(In reply to comment #4)
> (In reply to comment #3)
> I think we can use a DOM mutation event in the parent document.

Can't we just do setTimeout(function() {layoutTestController.notifyDone();}, 0) ?

[Ryosuke Niwa]

Comment on attachment 92929 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=92929&action=review

r- per various commets.

> LayoutTests/editing/pasteboard/paste-removing-iframe.html:9
> +<h1>PASS unless crash.</h1>

You need to be more descriptive here.  Also, you should just print PASS when WebKit didn't crash.

[Hajime Morrita]

Created attachment 93242 [details]
Patch

[Hajime Morrita]

Kent-san, Ryosuke, thank you for reviewing!
I updated the patch, in which I moved test script from child frame to parent frame.

> > LayoutTests/editing/pasteboard/paste-removing-iframe.html:9
> > +<h1>PASS unless crash.</h1>
> 
> You need to be more descriptive here.  Also, you should just print PASS when WebKit didn't crash.
Add more explanation as a comment (to make explanation small)
and simplify the text.

[Kent Tamura]

Comment on attachment 93242 [details]
Patch

ok

[Hajime Morrita]

Committed r86311: <http://trac.webkit.org/changeset/86311>

[Ademar Reis]

Revision r86311 cherry-picked into qtwebkit-2.2 with commit 25483fc <http://gitorious.org/webkit/qtwebkit/commit/25483fc>