Title: [258399] trunk
Revision
258399
Author
rn...@webkit.org
Date
2020-03-13 08:20:56 -0700 (Fri, 13 Mar 2020)

Log Message

Crash in TextIterator::node via TextManipulationController::replace
https://bugs.webkit.org/show_bug.cgi?id=209048

Reviewed by Wenson Hsieh.

Source/WebCore:

The crash was caused by item.start being null in TextManipulationController::replace for a paragraph
consisting of just an image because TextManipulationController::observeParagraphs never may never set
startOfCurrentParagraph in such a case (content.isTextContent can be false for an image!).

Fixed the bug by setting startOfCurrentParagraph to a position before the current content's node
when inserting a token for a RenderReplaced if it's null.

Test: TextManipulation.CompleteTextManipulationShouldReplaceContentsAroundParagraphWithJustImage

* editing/TextManipulationController.cpp:
(WebCore::TextManipulationController::observeParagraphs):

Tools:

Added a regression test.

* TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:
(TextManipulation.CompleteTextManipulationShouldReplaceContentsAroundParagraphWithJustImage):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (258398 => 258399)


--- trunk/Source/WebCore/ChangeLog	2020-03-13 14:55:37 UTC (rev 258398)
+++ trunk/Source/WebCore/ChangeLog	2020-03-13 15:20:56 UTC (rev 258399)
@@ -1,3 +1,22 @@
+2020-03-13  Ryosuke Niwa  <rn...@webkit.org>
+
+        Crash in TextIterator::node via TextManipulationController::replace
+        https://bugs.webkit.org/show_bug.cgi?id=209048
+
+        Reviewed by Wenson Hsieh.
+
+        The crash was caused by item.start being null in TextManipulationController::replace for a paragraph
+        consisting of just an image because TextManipulationController::observeParagraphs never may never set
+        startOfCurrentParagraph in such a case (content.isTextContent can be false for an image!).
+
+        Fixed the bug by setting startOfCurrentParagraph to a position before the current content's node
+        when inserting a token for a RenderReplaced if it's null.
+
+        Test: TextManipulation.CompleteTextManipulationShouldReplaceContentsAroundParagraphWithJustImage
+
+        * editing/TextManipulationController.cpp:
+        (WebCore::TextManipulationController::observeParagraphs):
+
 2020-03-13  Michael Catanzaro  <mcatanz...@gnome.org>
 
         -Wredundant-move in CSSPropertyParserHelpers.cpp

Modified: trunk/Source/WebCore/editing/TextManipulationController.cpp (258398 => 258399)


--- trunk/Source/WebCore/editing/TextManipulationController.cpp	2020-03-13 14:55:37 UTC (rev 258398)
+++ trunk/Source/WebCore/editing/TextManipulationController.cpp	2020-03-13 15:20:56 UTC (rev 258399)
@@ -279,6 +279,8 @@
         }
 
         if (content.isReplacedContent) {
+            if (startOfCurrentParagraph.isNull())
+                startOfCurrentParagraph = positionBeforeNode(content.node.get());
             tokensInCurrentParagraph.append(ManipulationToken { m_tokenIdentifier.generate(), "[]", true /* isExcluded */});
             continue;
         }

Modified: trunk/Tools/ChangeLog (258398 => 258399)


--- trunk/Tools/ChangeLog	2020-03-13 14:55:37 UTC (rev 258398)
+++ trunk/Tools/ChangeLog	2020-03-13 15:20:56 UTC (rev 258399)
@@ -1,3 +1,15 @@
+2020-03-13  Ryosuke Niwa  <rn...@webkit.org>
+
+        Crash in TextIterator::node via TextManipulationController::replace
+        https://bugs.webkit.org/show_bug.cgi?id=209048
+
+        Reviewed by Wenson Hsieh.
+
+        Added a regression test.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:
+        (TextManipulation.CompleteTextManipulationShouldReplaceContentsAroundParagraphWithJustImage):
+
 2020-03-13  Saam Barati  <sbar...@apple.com>
 
         Pull in JS tests from "Internal" when there are "Internal" tests in an adjacent directory

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm (258398 => 258399)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm	2020-03-13 14:55:37 UTC (rev 258398)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm	2020-03-13 15:20:56 UTC (rev 258399)
@@ -942,6 +942,42 @@
         "<div><span style=\"display: block\">hello, world</span></div></div>", [webView stringByEvaluatingJavaScript:@"document.body.innerHTML"]);
 }
 
+TEST(TextManipulation, CompleteTextManipulationShouldReplaceContentsAroundParagraphWithJustImage)
+{
+    auto delegate = adoptNS([[TextManipulationDelegate alloc] init]);
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 400, 400)]);
+    [webView _setTextManipulationDelegate:delegate.get()];
+
+    [webView synchronouslyLoadHTMLString:@"<!DOCTYPE html><html><body><div>heeey</div><div><img src=""
+
+    done = false;
+    [webView _startTextManipulationsWithConfiguration:nil completion:^{
+        done = true;
+    }];
+    TestWebKitAPI::Util::run(&done);
+
+    auto *items = [delegate items];
+    EXPECT_EQ(items.count, 3UL);
+    EXPECT_EQ(items[0].tokens.count, 1UL);
+    EXPECT_STREQ("heeey", items[0].tokens[0].content.UTF8String);
+    EXPECT_EQ(items[1].tokens.count, 1UL);
+    EXPECT_STREQ("[]", items[1].tokens[0].content.UTF8String);
+    EXPECT_EQ(items[2].tokens.count, 1UL);
+    EXPECT_STREQ("woorld", items[2].tokens[0].content.UTF8String);
+
+    done = false;
+    [webView _completeTextManipulationForItems:@[
+        (_WKTextManipulationItem *)createItem(items[0].identifier, { { items[0].tokens[0].identifier, @"hello" } }),
+        (_WKTextManipulationItem *)createItem(items[1].identifier, { { items[1].tokens[0].identifier, nil } }),
+        (_WKTextManipulationItem *)createItem(items[2].identifier, { { items[2].tokens[0].identifier, @"world" } }),
+    ] completion:^(NSArray<NSError *> *errors) {
+        EXPECT_EQ(errors, nil);
+        done = true;
+    }];
+    TestWebKitAPI::Util::run(&done);
+    EXPECT_WK_STREQ("<div>hello</div><div><img src="" [webView stringByEvaluatingJavaScript:@"document.body.innerHTML"]);
+}
+
 TEST(TextManipulation, CompleteTextManipulationShouldBatchItemCallback)
 {
     auto delegate = adoptNS([[TextManipulationDelegate alloc] init]);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to