Bug 221393 – Nullptr crash in ApplyStyleCommand::applyRelativeFontStyleChange

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

[Ryosuke Niwa]

Created attachment 419255 [details]
Test

e.g.

#0 0x6f78bda3e in WTF::OptionSet<WebCore::Node::NodeFlag>::containsAny(WTF::OptionSet<WebCore::Node::NodeFlag>) const+0xbe (WebCore.framework/Versions/A/WebCore:x86_64+0x1c7a3e)
#1 0x6f78bd919 in WTF::OptionSet<WebCore::Node::NodeFlag>::contains(WebCore::Node::NodeFlag) const+0xd9 (WebCore.framework/Versions/A/WebCore:x86_64+0x1c7919)
#2 0x6f78bd83c in WebCore::Node::hasNodeFlag(WebCore::Node::NodeFlag) const+0xc (WebCore.framework/Versions/A/WebCore:x86_64+0x1c783c)
#3 0x6f78bd82d in WebCore::Node::isHTMLElement() const+0xd (WebCore.framework/Versions/A/WebCore:x86_64+0x1c782d)
#4 0x6f78bd818 in WTF::TypeCastTraits<WebCore::HTMLElement const, WebCore::Node const, false>::isType(WebCore::Node const&)+0x8 (WebCore.framework/Versions/A/WebCore:x86_64+0x1c7818)
#5 0x6f78bd808 in WTF::TypeCastTraits<WebCore::HTMLElement const, WebCore::Node const, false>::isOfType(WebCore::Node const&)+0x8 (WebCore.framework/Versions/A/WebCore:x86_64+0x1c7808)
#6 0x6f8813868 in bool WTF::is<WebCore::HTMLElement, WebCore::Node>(WebCore::Node&)+0x8 (WebCore.framework/Versions/A/WebCore:x86_64+0x111d868)
#7 0x6fb227bee in WebCore::ApplyStyleCommand::applyRelativeFontStyleChange(WebCore::EditingStyle*)+0x94e (WebCore.framework/Versions/A/WebCore:x86_64+0x3b31bee)
#8 0x6fb226236 in WebCore::ApplyStyleCommand::doApply()+0x1a6 (WebCore.framework/Versions/A/WebCore:x86_64+0x3b30236)
#9 0x6fb220506 in WebCore::CompositeEditCommand::apply()+0x216 (WebCore.framework/Versions/A/WebCore:x86_64+0x3b2a506)
#10 0x6fb296860 in WebCore::Editor::applyStyle(WTF::RefPtr<WebCore::EditingStyle, WTF::RawPtrTraits<WebCore::EditingStyle>, WTF::DefaultRefDerefTraits<WebCore::EditingStyle> >&&, WebCore::EditAction, WebCore::Editor::ColorFilterMode)+0x420 (WebCore.framework/Versions/A/WebCore:x86_64+0x3ba0860)
#11 0x6fb2e1dfd in WebCore::applyCommandToFrame(WebCore::Frame&, WebCore::EditorCommandSource, WebCore::EditAction, WTF::Ref<WebCore::EditingStyle, WTF::RawPtrTraits<WebCore::EditingStyle> >&&)+0x10d (WebCore.framework/Versions/A/WebCore:x86_64+0x3bebdfd)
#12 0x6fb2e1c86 in WebCore::executeApplyStyle(WebCore::Frame&, WebCore::EditorCommandSource, WebCore::EditAction, WebCore::CSSPropertyID, WTF::String const&)+0xd6 (WebCore.framework/Versions/A/WebCore:x86_64+0x3bebc86)
#13 0x6fb2dc1f7 in WebCore::executeFontSizeDelta(WebCore::Frame&, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&)+0x17 (WebCore.framework/Versions/A/WebCore:x86_64+0x3be61f7)
#14 0x6fb2a23eb in WebCore::Editor::Command::execute(WTF::String const&, WebCore::Event*) const+0xdb (WebCore.framework/Versions/A/WebCore:x86_64+0x3bac3eb)
#15 0x6faf281f3 in WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&)+0xf3 (WebCore.framework/Versions/A/WebCore:x86_64+0x38321f3)
#16 0x6f81c3199 in WebCore::jsDocumentPrototypeFunction_execCommandBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDocument*)+0x469 (WebCore.framework/Versions/A/WebCore:x86_64+0xacd199)

<rdar://problem/73930150>

[Sergio Villar Senin]

Created attachment 420656 [details]
Patch

[Frédéric Wang (:fredw)]

Comment on attachment 420656 [details]
Patch

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

> Source/WebCore/editing/ApplyStyleCommand.cpp:396
> +            // also merge identical ones. As a consecuence, if |beyondEnd| is a span node located just after |node| it might be merged

consequence*

[Ryosuke Niwa]

Comment on attachment 420656 [details]
Patch

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

> Source/WebCore/editing/ApplyStyleCommand.cpp:381
> +    bool nodeSurpassedBeyondEndAfterElementMerging = false;

This variable name is rather verbose. Just reachedEnd is enough.

> Source/WebCore/editing/ApplyStyleCommand.cpp:382
> +    for (Node* node = startNode; node != beyondEnd && !nodeSurpassedBeyondEndAfterElementMerging; node = NodeTraversal::next(*node)) {

Can we use RefPtr by calling makeRefPtr while we're at it?

> Source/WebCore/editing/ApplyStyleCommand.cpp:395
> +            // The node tree might change while iterating this loop as surroundNodeRangeWithElement() creates a new node and could

I don't think this comment is necessary. It's pretty self evident from the code below.

[Sergio Villar Senin]

Committed r273380 (234507@main): <https://commits.webkit.org/234507@main>