Title: [294063] trunk
- Revision
- 294063
- Author
- [email protected]
- Date
- 2022-05-11 11:35:14 -0700 (Wed, 11 May 2022)
Log Message
[Webpage Translation] Avoid removing elements with no children during text manipulation
https://bugs.webkit.org/show_bug.cgi?id=240287
rdar://91882797
Reviewed by Tim Horton.
After invoking webpage translation on a particular website, the entire page becomes blank and unusable when
scrolling. This happens because of an uncaught _javascript_ exception that's thrown when this page's script
attempts to remove an element from its parent using `p.removeChild(c)`, where the node `p` is not a parent of
the given node `c`; this, in turn, happens because `TextManipulationController` has already unparented `c` from
`p` while performing text replacement during translation.
In this particular case, the former child node `c` is an empty `div` element with no text or children. As such,
it's unnecessary to flag this element for removal in the first place, since doing so isn't necessary to fill in
translated text.
We can avoid this issue by simply skipping over such nodes (i.e. containers that contain no text, no child
elements, and also are not replaced elements) to avoid this and similar compatibility issues that arise when
the DOM is mutated underneath the page, during translation.
Test: TextManipulation.CompleteTextManipulationSkipsEmptyContainers
* editing/TextManipulationController.cpp:
(WebCore::TextManipulationController::replace):
[Webpage Translation] Avoid removing elements with no children during text manipulation
https://bugs.webkit.org/show_bug.cgi?id=240287
rdar://91882797
Reviewed by Tim Horton.
Add a new API test to exercise the change.
* TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:
(TestWebKitAPI::TEST):
Canonical link: https://commits.webkit.org/250467@main
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (294062 => 294063)
--- trunk/Source/WebCore/ChangeLog 2022-05-11 18:18:49 UTC (rev 294062)
+++ trunk/Source/WebCore/ChangeLog 2022-05-11 18:35:14 UTC (rev 294063)
@@ -1,3 +1,30 @@
+2022-05-10 Wenson Hsieh <[email protected]>
+
+ [Webpage Translation] Avoid removing elements with no children during text manipulation
+ https://bugs.webkit.org/show_bug.cgi?id=240287
+ rdar://91882797
+
+ Reviewed by Tim Horton.
+
+ After invoking webpage translation on a particular website, the entire page becomes blank and unusable when
+ scrolling. This happens because of an uncaught _javascript_ exception that's thrown when this page's script
+ attempts to remove an element from its parent using `p.removeChild(c)`, where the node `p` is not a parent of
+ the given node `c`; this, in turn, happens because `TextManipulationController` has already unparented `c` from
+ `p` while performing text replacement during translation.
+
+ In this particular case, the former child node `c` is an empty `div` element with no text or children. As such,
+ it's unnecessary to flag this element for removal in the first place, since doing so isn't necessary to fill in
+ translated text.
+
+ We can avoid this issue by simply skipping over such nodes (i.e. containers that contain no text, no child
+ elements, and also are not replaced elements) to avoid this and similar compatibility issues that arise when
+ the DOM is mutated underneath the page, during translation.
+
+ Test: TextManipulation.CompleteTextManipulationSkipsEmptyContainers
+
+ * editing/TextManipulationController.cpp:
+ (WebCore::TextManipulationController::replace):
+
2022-05-11 Youenn Fablet <[email protected]>
Add a webshare quirk for youtube
Modified: trunk/Source/WebCore/editing/TextManipulationController.cpp (294062 => 294063)
--- trunk/Source/WebCore/editing/TextManipulationController.cpp 2022-05-11 18:18:49 UTC (rev 294062)
+++ trunk/Source/WebCore/editing/TextManipulationController.cpp 2022-05-11 18:35:14 UTC (rev 294063)
@@ -791,10 +791,14 @@
auto content = iterator.currentContent();
ASSERT(content.node);
+ bool isReplacedOrTextContent = content.isReplacedContent || content.isTextContent;
+ if (!isReplacedOrTextContent && is<ContainerNode>(*content.node) && !content.node->hasChildNodes() && content.text.isEmpty())
+ continue;
+
lastChildOfCommonAncestorInRange = content.node;
nodesToRemove.add(*content.node);
- if (!content.isReplacedContent && !content.isTextContent)
+ if (!isReplacedOrTextContent)
continue;
Vector<ManipulationToken> tokensInCurrentNode;
Modified: trunk/Tools/ChangeLog (294062 => 294063)
--- trunk/Tools/ChangeLog 2022-05-11 18:18:49 UTC (rev 294062)
+++ trunk/Tools/ChangeLog 2022-05-11 18:35:14 UTC (rev 294063)
@@ -1,3 +1,16 @@
+2022-05-10 Wenson Hsieh <[email protected]>
+
+ [Webpage Translation] Avoid removing elements with no children during text manipulation
+ https://bugs.webkit.org/show_bug.cgi?id=240287
+ rdar://91882797
+
+ Reviewed by Tim Horton.
+
+ Add a new API test to exercise the change.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:
+ (TestWebKitAPI::TEST):
+
2022-05-10 Jonathan Bedard <[email protected]>
[git-webkit] Fail quickly for invalid bugzilla credentials
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm (294062 => 294063)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm 2022-05-11 18:18:49 UTC (rev 294062)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm 2022-05-11 18:35:14 UTC (rev 294063)
@@ -3414,6 +3414,46 @@
EXPECT_WK_STREQ("<span class=\"hidden\">hello</span>", [webView stringByEvaluatingJavaScript:@"document.body.innerHTML"]);
}
+TEST(TextManipulation, CompleteTextManipulationSkipsEmptyContainers)
+{
+ auto delegate = adoptNS([[TextManipulationDelegate alloc] init]);
+ auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 400, 400)]);
+ [webView _setTextManipulationDelegate:delegate.get()];
+ [webView synchronouslyLoadHTMLString:@"<section><a href=''>Guten<img width='50' src=''></section>"
+ "<section><div id='target'></div><div><a href=''>Tag</a></div></section>"];
+
+ 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, 3UL);
+ EXPECT_WK_STREQ("Guten", items[0].tokens[0].content);
+ EXPECT_WK_STREQ("[]", items[0].tokens[1].content);
+ EXPECT_WK_STREQ("Tag", items[0].tokens[2].content);
+
+ auto replacement = createItem(items[0].identifier, {
+ { items[0].tokens[0].identifier, @"Good" },
+ { items[0].tokens[2].identifier, @"Day" }
+ });
+
+ done = false;
+ [webView _completeTextManipulationForItems:@[replacement.get()] completion:^(NSArray<NSError *> *errors) {
+ EXPECT_EQ(errors.count, 0UL);
+ done = true;
+ }];
+ TestWebKitAPI::Util::run(&done);
+
+ EXPECT_WK_STREQ("SECTION", [webView stringByEvaluatingJavaScript:@"document.getElementById('target').parentElement.tagName"]);
+ NSArray<NSString *> *textContents = [webView objectByEvaluatingJavaScript:@"Array.from(document.links).map(a => a.textContent)"];
+ EXPECT_EQ(textContents.count, 2U);
+ EXPECT_WK_STREQ("Good", textContents.firstObject);
+ EXPECT_WK_STREQ("Day", textContents.lastObject);
+}
+
TEST(TextManipulation, TextManipulationTokenDebugDescription)
{
auto token = adoptNS([[_WKTextManipulationToken alloc] init]);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
