Title: [262879] trunk
Revision
262879
Author
sihui_...@apple.com
Date
2020-06-10 17:28:06 -0700 (Wed, 10 Jun 2020)

Log Message

Text manipulation does not observe inserted elements that are invisible
https://bugs.webkit.org/show_bug.cgi?id=213057
<rdar://problem/63768253>

Reviewed by Wenson Hsieh.

Source/WebCore:

TextManipulationController gets notification when renderer of an element is created and starts observing the
element. It currently sets the observing range to be the visible start position and visible end position of the
element. When the invisible content becomes visible later, TextManipulationController does not get notification
and will miss the content. Therefore, TextManipulationController should use the actual start and end positions
of the element for range.

Test: TextManipulation.StartTextManipulationFindsInsertedClippedText

* editing/TextManipulationController.cpp:
(WebCore::makeHashablePositionRange):
(WebCore::TextManipulationController::scheduleObservationUpdate):

Tools:

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

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (262878 => 262879)


--- trunk/Source/WebCore/ChangeLog	2020-06-11 00:15:05 UTC (rev 262878)
+++ trunk/Source/WebCore/ChangeLog	2020-06-11 00:28:06 UTC (rev 262879)
@@ -1,3 +1,23 @@
+2020-06-10  Sihui Liu  <sihui_...@apple.com>
+
+        Text manipulation does not observe inserted elements that are invisible
+        https://bugs.webkit.org/show_bug.cgi?id=213057
+        <rdar://problem/63768253>
+
+        Reviewed by Wenson Hsieh.
+
+        TextManipulationController gets notification when renderer of an element is created and starts observing the 
+        element. It currently sets the observing range to be the visible start position and visible end position of the 
+        element. When the invisible content becomes visible later, TextManipulationController does not get notification 
+        and will miss the content. Therefore, TextManipulationController should use the actual start and end positions 
+        of the element for range.
+
+        Test: TextManipulation.StartTextManipulationFindsInsertedClippedText
+
+        * editing/TextManipulationController.cpp:
+        (WebCore::makeHashablePositionRange):
+        (WebCore::TextManipulationController::scheduleObservationUpdate):
+
 2020-06-10  Geoffrey Garen  <gga...@apple.com>
 
         Some style improvements to main thread code

Modified: trunk/Source/WebCore/editing/TextManipulationController.cpp (262878 => 262879)


--- trunk/Source/WebCore/editing/TextManipulationController.cpp	2020-06-11 00:15:05 UTC (rev 262878)
+++ trunk/Source/WebCore/editing/TextManipulationController.cpp	2020-06-11 00:28:06 UTC (rev 262879)
@@ -507,9 +507,9 @@
     return { position.anchorNode(), static_cast<unsigned>(position.anchorType()), position.anchorType() == Position::PositionIsOffsetInAnchor ? position.offsetInContainerNode() : 0 };
 }
 
-static const std::pair<PositionTuple, PositionTuple> makeHashablePositionRange(const VisiblePosition& start, const VisiblePosition& end)
+static const std::pair<PositionTuple, PositionTuple> makeHashablePositionRange(const Position& start, const Position& end)
 {
-    return { makePositionTuple(start.deepEquivalent()), makePositionTuple(end.deepEquivalent()) };
+    return { makePositionTuple(start), makePositionTuple(end) };
 }
 
 void TextManipulationController::scheduleObservationUpdate()
@@ -537,8 +537,8 @@
 
         HashSet<std::pair<PositionTuple, PositionTuple>> paragraphSets;
         for (auto& element : filteredElements) {
-            auto start = startOfParagraph(firstPositionInOrBeforeNode(element.ptr()));
-            auto end = endOfParagraph(lastPositionInOrAfterNode(element.ptr()));
+            auto start = firstPositionInOrBeforeNode(element.ptr());
+            auto end = lastPositionInOrAfterNode(element.ptr());
 
             if (start.isNull() || end.isNull())
                 continue;
@@ -549,9 +549,9 @@
 
             auto* controller = weakThis.get();
             if (!controller)
-                return; // Finding the start/end of paragraph may have updated layout & executed arbitrary scripts.
+                return;
 
-            controller->observeParagraphs(start.deepEquivalent(), end.deepEquivalent());
+            controller->observeParagraphs(start, end);
         }
         controller->flushPendingItemsForCallback();
     });

Modified: trunk/Tools/ChangeLog (262878 => 262879)


--- trunk/Tools/ChangeLog	2020-06-11 00:15:05 UTC (rev 262878)
+++ trunk/Tools/ChangeLog	2020-06-11 00:28:06 UTC (rev 262879)
@@ -1,3 +1,14 @@
+2020-06-10  Sihui Liu  <sihui_...@apple.com>
+
+        Text manipulation does not observe inserted elements that are invisible
+        https://bugs.webkit.org/show_bug.cgi?id=213057
+        <rdar://problem/63768253>
+
+        Reviewed by Wenson Hsieh.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:
+        (TestWebKitAPI::TEST):
+
 2020-06-10  Geoffrey Garen  <gga...@apple.com>
 
         Some style improvements to main thread code

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm (262878 => 262879)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm	2020-06-11 00:15:05 UTC (rev 262878)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm	2020-06-11 00:28:06 UTC (rev 262879)
@@ -602,6 +602,60 @@
     EXPECT_WK_STREQ("More text", items[1].tokens[0].content);
 }
 
+TEST(TextManipulation, StartTextManipulationFindsInsertedClippedText)
+{
+    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>"
+        "<head>"
+        "    <style>"
+        "        span { overflow: hidden; width: 200px; height: 0; }"
+        "        p { visibility: hidden; }"
+        "    </style>"
+        "</head>"
+        "<body>"
+        "    <div>hello, world</div>"
+        "</body>"];
+
+    done = false;
+    [webView _startTextManipulationsWithConfiguration:nil completion:^{
+        done = true;
+    }];
+    TestWebKitAPI::Util::run(&done);
+
+    auto *items = [delegate items];
+    EXPECT_EQ(items.count, 1UL);
+
+    done = false;
+    delegate.get().itemCallback = ^(_WKTextManipulationItem *item) {
+        if (items.count == 2)
+            done = true;
+    };
+    [webView stringByEvaluatingJavaScript:@"var beforeElement = document.createElement('span');"
+        "beforeElement.innerHTML='before';"
+        "document.querySelector('div').before(beforeElement);"];
+    TestWebKitAPI::Util::run(&done);
+
+    done = false;
+    delegate.get().itemCallback = ^(_WKTextManipulationItem *item) {
+        if (items.count == 3)
+            done = true;
+    };
+    [webView stringByEvaluatingJavaScript:@"var afterElement = document.createElement('p');"
+        "afterElement.innerHTML='after';"
+        "document.querySelector('div').after(afterElement)"];
+    TestWebKitAPI::Util::run(&done);
+
+    EXPECT_EQ(items[0].tokens.count, 1UL);
+    EXPECT_WK_STREQ("hello, world", items[0].tokens[0].content);
+    EXPECT_EQ(items[1].tokens.count, 1UL);
+    EXPECT_WK_STREQ("before", items[1].tokens[0].content);
+    EXPECT_EQ(items[2].tokens.count, 1UL);
+    EXPECT_WK_STREQ("after", items[2].tokens[0].content);
+}
+
 TEST(TextManipulation, StartTextManipulationTreatsInlineBlockLinksAndButtonsAsParagraphs)
 {
     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