- Revision
- 274865
- Author
- [email protected]
- Date
- 2021-03-23 06:55:33 -0700 (Tue, 23 Mar 2021)
Log Message
Nullptr deref in WebCore::ApplyStyleCommand::applyRelativeFontStyleChange
https://bugs.webkit.org/show_bug.cgi?id=223364
Patch by Frédéric Wang <[email protected]> on 2021-03-23
Reviewed by Ryosuke Niwa.
Source/WebCore:
When the editing code creates a span to apply font style change, it may not have editable
style if the document sets extra style (e.g. user-select: all). This is causing a debug
ASSERT in AppendNodeCommand::AppendNodeCommand when the span is inserted and a nullptr
dereference later in release mode. This patch ensures that we skip the font style change
when that happens.
Test: editing/style/apply-font-style-change-crash.html
* editing/ApplyStyleCommand.cpp:
(WebCore::ApplyStyleCommand::applyRelativeFontStyleChange): Skip the font style change if
the span insertion failed.
(WebCore::ApplyStyleCommand::surroundNodeRangeWithElement): After inserting the new element,
ensure that the conditions from the ASSERT of AppendNodeCommand::AppendNodeCommand hold and
return failure if they don't.
* editing/ApplyStyleCommand.h: Return a boolean indicating success.
LayoutTests:
Add regression test.
* editing/style/apply-font-style-change-crash-expected.txt: Added.
* editing/style/apply-font-style-change-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (274864 => 274865)
--- trunk/LayoutTests/ChangeLog 2021-03-23 13:39:03 UTC (rev 274864)
+++ trunk/LayoutTests/ChangeLog 2021-03-23 13:55:33 UTC (rev 274865)
@@ -1,5 +1,17 @@
2021-03-23 Frédéric Wang <[email protected]>
+ Nullptr deref in WebCore::ApplyStyleCommand::applyRelativeFontStyleChange
+ https://bugs.webkit.org/show_bug.cgi?id=223364
+
+ Reviewed by Ryosuke Niwa.
+
+ Add regression test.
+
+ * editing/style/apply-font-style-change-crash-expected.txt: Added.
+ * editing/style/apply-font-style-change-crash.html: Added.
+
+2021-03-23 Frédéric Wang <[email protected]>
+
Nullopt in DOMSelection::getRangeAt
https://bugs.webkit.org/show_bug.cgi?id=223361
Added: trunk/LayoutTests/editing/style/apply-font-style-change-crash-expected.txt (0 => 274865)
--- trunk/LayoutTests/editing/style/apply-font-style-change-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/style/apply-font-style-change-crash-expected.txt 2021-03-23 13:55:33 UTC (rev 274865)
@@ -0,0 +1,2 @@
+CONSOLE MESSAGE: This test passes if it does not crash.
+_onload_ = () => { if (window.testRunner) testRunner.dumpAsText(); console.log("This test passes if it does not crash."); document.execCommand('SelectAll'); document.designMode = 'on'; document.execCommand('FontSizeDelta', false, '1'); };
Added: trunk/LayoutTests/editing/style/apply-font-style-change-crash.html (0 => 274865)
--- trunk/LayoutTests/editing/style/apply-font-style-change-crash.html (rev 0)
+++ trunk/LayoutTests/editing/style/apply-font-style-change-crash.html 2021-03-23 13:55:33 UTC (rev 274865)
@@ -0,0 +1,25 @@
+<head>
+ <meta>
+ <style>
+ meta {
+ -webkit-user-modify: blah;
+ border-inline-style: solid;
+ }
+ :nth-child(2) {
+ -webkit-user-select: all;
+ }
+ head, script {
+ all: unset;
+ }
+ </style>
+ <script>
+ _onload_ = () => {
+ if (window.testRunner)
+ testRunner.dumpAsText();
+ console.log("This test passes if it does not crash.");
+ document.execCommand('SelectAll');
+ document.designMode = 'on';
+ document.execCommand('FontSizeDelta', false, '1');
+ };
+ </script>
+</head>
Modified: trunk/Source/WebCore/ChangeLog (274864 => 274865)
--- trunk/Source/WebCore/ChangeLog 2021-03-23 13:39:03 UTC (rev 274864)
+++ trunk/Source/WebCore/ChangeLog 2021-03-23 13:55:33 UTC (rev 274865)
@@ -1,3 +1,26 @@
+2021-03-23 Frédéric Wang <[email protected]>
+
+ Nullptr deref in WebCore::ApplyStyleCommand::applyRelativeFontStyleChange
+ https://bugs.webkit.org/show_bug.cgi?id=223364
+
+ Reviewed by Ryosuke Niwa.
+
+ When the editing code creates a span to apply font style change, it may not have editable
+ style if the document sets extra style (e.g. user-select: all). This is causing a debug
+ ASSERT in AppendNodeCommand::AppendNodeCommand when the span is inserted and a nullptr
+ dereference later in release mode. This patch ensures that we skip the font style change
+ when that happens.
+
+ Test: editing/style/apply-font-style-change-crash.html
+
+ * editing/ApplyStyleCommand.cpp:
+ (WebCore::ApplyStyleCommand::applyRelativeFontStyleChange): Skip the font style change if
+ the span insertion failed.
+ (WebCore::ApplyStyleCommand::surroundNodeRangeWithElement): After inserting the new element,
+ ensure that the conditions from the ASSERT of AppendNodeCommand::AppendNodeCommand hold and
+ return failure if they don't.
+ * editing/ApplyStyleCommand.h: Return a boolean indicating success.
+
2021-03-23 Zalan Bujtas <[email protected]>
[ContentChangeObserver] Unable to view state details on CDC COVID map
Modified: trunk/Source/WebCore/editing/ApplyStyleCommand.cpp (274864 => 274865)
--- trunk/Source/WebCore/editing/ApplyStyleCommand.cpp 2021-03-23 13:39:03 UTC (rev 274864)
+++ trunk/Source/WebCore/editing/ApplyStyleCommand.cpp 2021-03-23 13:55:33 UTC (rev 274865)
@@ -391,7 +391,8 @@
// Last styled node was not parent node of this text node, but we wish to style this
// text node. To make this possible, add a style span to surround this text node.
auto span = createStyleSpanElement(document());
- surroundNodeRangeWithElement(*node, *node, span.copyRef());
+ if (!surroundNodeRangeWithElement(*node, *node, span.copyRef()))
+ continue;
reachedEnd = node->isDescendantOf(beyondEnd);
element = WTFMove(span);
} else {
@@ -1305,12 +1306,16 @@
return true;
}
-void ApplyStyleCommand::surroundNodeRangeWithElement(Node& startNode, Node& endNode, Ref<Element>&& elementToInsert)
+bool ApplyStyleCommand::surroundNodeRangeWithElement(Node& startNode, Node& endNode, Ref<Element>&& elementToInsert)
{
Ref<Node> protectedStartNode = startNode;
Ref<Element> element = WTFMove(elementToInsert);
insertNodeBefore(element.copyRef(), startNode);
+ if (!element->isContentRichlyEditable()) {
+ removeNode(element);
+ return false;
+ }
RefPtr<Node> node = &startNode;
while (node) {
@@ -1340,6 +1345,7 @@
// FIXME: We should probably call updateStartEnd if the start or end was in the node
// range so that the endingSelection() is canonicalized. See the comments at the end of
// VisibleSelection::validate().
+ return true;
}
static String joinWithSpace(const String& a, const String& b)
Modified: trunk/Source/WebCore/editing/ApplyStyleCommand.h (274864 => 274865)
--- trunk/Source/WebCore/editing/ApplyStyleCommand.h 2021-03-23 13:39:03 UTC (rev 274864)
+++ trunk/Source/WebCore/editing/ApplyStyleCommand.h 2021-03-23 13:55:33 UTC (rev 274865)
@@ -109,7 +109,7 @@
bool mergeEndWithNextIfIdentical(const Position& start, const Position& end);
void cleanupUnstyledAppleStyleSpans(ContainerNode* dummySpanAncestor);
- void surroundNodeRangeWithElement(Node& start, Node& end, Ref<Element>&&);
+ bool surroundNodeRangeWithElement(Node& start, Node& end, Ref<Element>&&);
float computedFontSize(Node*);
void joinChildTextNodes(Node*, const Position& start, const Position& end);
