Bug 221393 - Nullptr crash in ApplyStyleCommand::applyRelativeFontStyleChange
Summary: Nullptr crash in ApplyStyleCommand::applyRelativeFontStyleChange
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-02-03 23:57 PST by Ryosuke Niwa
Modified: 2021-02-24 19:28 PST (History)
10 users (show)

See Also:


Attachments
Test (431 bytes, text/html)
2021-02-03 23:57 PST, Ryosuke Niwa
no flags Details
Patch (5.45 KB, patch)
2021-02-17 09:28 PST, Sergio Villar Senin
rniwa: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2021-02-03 23:57:43 PST
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>
Comment 1 Sergio Villar Senin 2021-02-17 09:28:03 PST
Created attachment 420656 [details]
Patch
Comment 2 Frédéric Wang (:fredw) 2021-02-18 08:38:16 PST
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*
Comment 3 Ryosuke Niwa 2021-02-22 20:06:00 PST
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.
Comment 4 Sergio Villar Senin 2021-02-24 04:54:11 PST
Committed r273380 (234507@main): <https://commits.webkit.org/234507@main>