Bug 223639 - Crash in WebCore::positionInParentBeforeNode(..)
Summary: Crash in WebCore::positionInParentBeforeNode(..)
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
Keywords: InRadar
Depends on:
Reported: 2021-03-23 09:15 PDT by Venky Dass
Modified: 2021-03-30 18:36 PDT (History)
6 users (show)

See Also:

Patch (4.31 KB, patch)
2021-03-23 10:02 PDT, Venky Dass
no flags Details | Formatted Diff | Diff
Patch (4.20 KB, patch)
2021-03-25 10:55 PDT, Venky Dass
no flags Details | Formatted Diff | Diff
Patch (4.32 KB, patch)
2021-03-30 11:47 PDT, Venky Dass
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Venky Dass 2021-03-23 09:15:42 PDT
Crash when there is a selectall and insert image is executed.

    #0 0x2ec93327a in WebCore::positionInParentBeforeNode(WebCore::Node*)+0x4a (/Users/user/dev/wk/ak/OpenSource/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0x493127a)
    #1 0x2eccc8314 in WebCore::ReplaceSelectionCommand::doApply()+0x2644 (/Users/user/dev/wk/ak/OpenSource/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0x4cc6314)
    #2 0x2ecaa3860 in WebCore::CompositeEditCommand::apply()+0x250 (/Users/user/dev/wk/ak/OpenSource/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0x4aa1860)
    #3 0x2ecc4923b in WebCore::executeInsertFragment(WebCore::Frame&, WTF::Ref<WebCore::DocumentFragment, WTF::RawPtrTraits<WebCore::DocumentFragment> >&&)+0x1ab (/Users/user/dev/wk/ak/OpenSource/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0x4c4723b)
    #4 0x2ecc496f7 in WebCore::executeInsertNode(WebCore::Frame&, WTF::Ref<WebCore::Node, WTF::RawPtrTraits<WebCore::Node> >&&)+0x197 (/Users/user/dev/wk/ak/OpenSource/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0x4c476f7)
    #5 0x2ecc3ec43 in WebCore::executeInsertImage(WebCore::Frame&, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&)+0x1c3 (/Users/user/dev/wk/ak/OpenSource/WebKitBuild/Release/WebCore.framework/Versions/A/WebCore:x86_64+0x4c3cc43)
    #6 0x2ec72668e in WebCore::Document::execCommand(WTF::String const&, bool, WTF

Comment 1 Venky Dass 2021-03-23 10:02:24 PDT
Created attachment 424032 [details]
Comment 2 Ryosuke Niwa 2021-03-23 17:04:48 PDT
Comment on attachment 424032 [details]

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

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1144
> +        Node* br = endingSelection().start().deprecatedNode();
> +        if (br != nullptr) {

Please use RefPtr. Also we don't compare against nullptr:
So this should be:
if (auto br = makeRefPtr(endingSelection().start().deprecatedNode()))

> Source/WebCore/editing/ReplaceSelectionCommand.cpp:1146
> +            // Insert content between the two blockquotes, but remove the br (since it was just a placeholder).

Can we just delete this code? It's pretty obvious from the code below.
There is no need to repeat the code in a comment.

> LayoutTests/editing/inserting/edit-style-and-insert-image.html:8
> +testRunner.dumpAsText();

Please indent this by 4 spaces for better readability.
Comment 3 Venky Dass 2021-03-25 10:55:07 PDT
Created attachment 424258 [details]
Comment 4 Ryosuke Niwa 2021-03-27 04:07:56 PDT
Comment on attachment 424258 [details]

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

> LayoutTests/editing/inserting/edit-style-and-insert-image.html:4
> +<p>This tests Pass.</p>

This text isn't printed in the output because of the code we're running.
Also, we want something likely more descriptive than that. Something like this:
This tests InsertImage after setting content property on the document element.<br>
WebKit should not crash.<br>

> LayoutTests/editing/inserting/edit-style-and-insert-image.html:11
> +document.execCommand('InsertImage');

Let's reset the style on documentElement here.
Comment 5 Venky Dass 2021-03-30 11:47:32 PDT
Created attachment 424668 [details]
Comment 6 EWS 2021-03-30 18:36:00 PDT
Committed r275253: <https://commits.webkit.org/r275253>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424668 [details].