Title: [265508] trunk
- Revision
- 265508
- Author
- [email protected]
- Date
- 2020-08-11 11:14:26 -0700 (Tue, 11 Aug 2020)
Log Message
Text manipulation crashes when replacing element with img role
https://bugs.webkit.org/show_bug.cgi?id=215344
Reviewed by Wenson Hsieh.
Source/WebCore:
positionInParentAfterNode and positionInParentBeforeNode do not return Position with Nodes that are ignored by
editing. Element with img role is one of such Nodes. However, TextManipulationController can manipulate content
of children of the ignored Nodes (see the newly added test). Therefore, we'd better not use
positionInParentBeforeNode or positionInParentBeforeNode in TextManipulationController::replace, because
insertion position can be inside a ignored Node.
API Test: TextManipulation.CompleteTextManipulationShouldReplaceContentIgnoredByEditing
* editing/TextManipulationController.cpp:
(WebCore::TextManipulationController::replace):
Tools:
* TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:
(TestWebKitAPI::TEST):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (265507 => 265508)
--- trunk/Source/WebCore/ChangeLog 2020-08-11 18:12:31 UTC (rev 265507)
+++ trunk/Source/WebCore/ChangeLog 2020-08-11 18:14:26 UTC (rev 265508)
@@ -1,3 +1,21 @@
+2020-08-11 Sihui Liu <[email protected]>
+
+ Text manipulation crashes when replacing element with img role
+ https://bugs.webkit.org/show_bug.cgi?id=215344
+
+ Reviewed by Wenson Hsieh.
+
+ positionInParentAfterNode and positionInParentBeforeNode do not return Position with Nodes that are ignored by
+ editing. Element with img role is one of such Nodes. However, TextManipulationController can manipulate content
+ of children of the ignored Nodes (see the newly added test). Therefore, we'd better not use
+ positionInParentBeforeNode or positionInParentBeforeNode in TextManipulationController::replace, because
+ insertion position can be inside a ignored Node.
+
+ API Test: TextManipulation.CompleteTextManipulationShouldReplaceContentIgnoredByEditing
+
+ * editing/TextManipulationController.cpp:
+ (WebCore::TextManipulationController::replace):
+
2020-08-11 Kenneth Russell <[email protected]>
[WebGL2] expando-loss and expando-loss-2 conformance tests are failing
Modified: trunk/Source/WebCore/editing/TextManipulationController.cpp (265507 => 265508)
--- trunk/Source/WebCore/editing/TextManipulationController.cpp 2020-08-11 18:12:31 UTC (rev 265507)
+++ trunk/Source/WebCore/editing/TextManipulationController.cpp 2020-08-11 18:14:26 UTC (rev 265508)
@@ -861,14 +861,12 @@
updateInsertions(lastTopDownPath, topDownPath, replacementNode.get(), reusedOriginalNodes, insertions);
}
- auto end = lastChildOfCommonAncestorInRange ? positionInParentAfterNode(lastChildOfCommonAncestorInRange.get()) : positionAfterNode(commonAncestor.get());
RefPtr<Node> node = item.end.firstNode();
- RefPtr<Node> endNode = end.firstNode();
- if (node && node != endNode) {
+ if (node && lastChildOfCommonAncestorInRange->contains(node.get())) {
auto topDownPath = getPath(commonAncestor.get(), node->parentNode());
updateInsertions(lastTopDownPath, topDownPath, nullptr, reusedOriginalNodes, insertions);
}
- while (node != endNode) {
+ while (lastChildOfCommonAncestorInRange->contains(node.get())) {
Ref<Node> parentNode = *node->parentNode();
while (!lastTopDownPath.isEmpty() && lastTopDownPath.last().first.ptr() != parentNode.ptr())
lastTopDownPath.removeLast();
@@ -878,9 +876,7 @@
node = NodeTraversal::next(*node);
}
- Position insertionPoint = positionBeforeNode(firstContentNode.get()).parentAnchoredEquivalent();
- while (insertionPoint.containerNode() != commonAncestor)
- insertionPoint = positionInParentBeforeNode(insertionPoint.containerNode());
+ RefPtr<Node> insertionPointNode = lastChildOfCommonAncestorInRange->nextSibling();
for (auto& node : nodesToRemove)
node->remove();
@@ -887,12 +883,11 @@
for (auto& insertion : insertions) {
auto parentContainer = insertion.parentIfDifferentFromCommonAncestor;
- if (!insertion.parentIfDifferentFromCommonAncestor) {
- parentContainer = insertionPoint.containerNode();
- parentContainer->insertBefore(insertion.child, insertionPoint.computeNodeAfterPosition());
- insertionPoint = positionInParentAfterNode(insertion.child.ptr());
+ if (!parentContainer) {
+ parentContainer = commonAncestor;
+ parentContainer->insertBefore(insertion.child, insertionPointNode.get());
} else
- insertion.parentIfDifferentFromCommonAncestor->appendChild(insertion.child);
+ parentContainer->appendChild(insertion.child);
if (auto* box = parentContainer->renderBox()) {
if (!box->hasVisualOverflow())
Modified: trunk/Tools/ChangeLog (265507 => 265508)
--- trunk/Tools/ChangeLog 2020-08-11 18:12:31 UTC (rev 265507)
+++ trunk/Tools/ChangeLog 2020-08-11 18:14:26 UTC (rev 265508)
@@ -1,3 +1,13 @@
+2020-08-11 Sihui Liu <[email protected]>
+
+ Text manipulation crashes when replacing element with img role
+ https://bugs.webkit.org/show_bug.cgi?id=215344
+
+ Reviewed by Wenson Hsieh.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:
+ (TestWebKitAPI::TEST):
+
2020-08-11 Philippe Normand <[email protected]>
[GTK][WPE] Add MiniBrowser wrapper env vars and white-list MESA env vars in flatpak
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm (265507 => 265508)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm 2020-08-11 18:12:31 UTC (rev 265507)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm 2020-08-11 18:14:26 UTC (rev 265508)
@@ -3082,6 +3082,48 @@
EXPECT_WK_STREQ("<span>Hello</span><span>World</span>", [webView stringByEvaluatingJavaScript:@"document.body.innerHTML"]);
}
+TEST(TextManipulation, CompleteTextManipulationShouldReplaceContentIgnoredByEditing)
+{
+ 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>"
+ "<body>"
+ "<div role='img'>"
+ "images"
+ "<a>link</a>"
+ "<img src=''>"
+ "<img src=''>"
+ "</div>"
+ "</body>"];
+
+ done = false;
+ [webView _startTextManipulationsWithConfiguration:nil completion:^{
+ done = true;
+ }];
+ TestWebKitAPI::Util::run(&done);
+
+ auto items = [delegate items];
+ EXPECT_EQ(items.count, 1UL);
+ EXPECT_EQ(items[0].tokens.count, 2UL);
+ EXPECT_WK_STREQ("images", items[0].tokens[0].content);
+ EXPECT_WK_STREQ("link", items[0].tokens[1].content);
+
+ done = false;
+ [webView _completeTextManipulationForItems:@[
+ createItem(items[0].identifier, {
+ { items[0].tokens[0].identifier, @"Images" },
+ { items[0].tokens[1].identifier, @"Link" }
+ }).get(),
+ ] completion:^(NSArray<NSError *> *errors) {
+ EXPECT_EQ(errors, nil);
+ done = true;
+ }];
+ TestWebKitAPI::Util::run(&done);
+
+ EXPECT_WK_STREQ("<div role=\"img\">Images<a>Link</a><img src="" src="" [webView stringByEvaluatingJavaScript:@"document.body.innerHTML"]);
+}
+
TEST(TextManipulation, TextManipulationTokenDebugDescription)
{
auto token = adoptNS([[_WKTextManipulationToken alloc] init]);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
