Modified: trunk/Source/WebCore/ChangeLog (260677 => 260678)
--- trunk/Source/WebCore/ChangeLog 2020-04-24 22:49:18 UTC (rev 260677)
+++ trunk/Source/WebCore/ChangeLog 2020-04-24 23:18:05 UTC (rev 260678)
@@ -1,3 +1,25 @@
+2020-04-24 Wenson Hsieh <[email protected]>
+
+ Make some more adjustments to TextManipulationController's paragraph boundary heuristic
+ https://bugs.webkit.org/show_bug.cgi?id=210993
+ <rdar://problem/61571299>
+
+ Reviewed by Tim Horton.
+
+ Adjust the heuristic added in r260583 to account for a few more common scenarios where we currently consider
+ text as a part of the same paragraph. This can lead to many issues during text manipulation where text is moved
+ between these elements, when it should not be.
+
+ The new scenarios include block and inline-block links, as well as button elements.
+
+ Test: TextManipulation.StartTextManipulationTreatsInlineBlockLinksAndButtonsAsParagraphs
+
+ * editing/TextManipulationController.cpp:
+ (WebCore::TextManipulationController::observeParagraphs):
+
+ Additionally rename "paragraph boundary element" to "item boundary element", to avoid colliding with the
+ existing notion of paragraph boundaries in editing code.
+
2020-04-24 Christopher Reid <[email protected]>
[Win] Bundle Inspector Resources in Release builds
Modified: trunk/Source/WebCore/editing/TextManipulationController.cpp (260677 => 260678)
--- trunk/Source/WebCore/editing/TextManipulationController.cpp 2020-04-24 22:49:18 UTC (rev 260677)
+++ trunk/Source/WebCore/editing/TextManipulationController.cpp 2020-04-24 23:18:05 UTC (rev 260678)
@@ -274,13 +274,21 @@
Vector<ManipulationToken> tokensInCurrentParagraph;
Position startOfCurrentParagraph;
Position endOfCurrentParagraph;
- RefPtr<Element> enclosingParagraphElement;
+ RefPtr<Element> enclosingItemBoundaryElement;
- auto isEnclosingParagraphElement = [](const Element& element) {
- if (element.hasTagName(HTMLNames::liTag)) {
- auto* renderer = element.renderer();
- return renderer && renderer->style().display() == DisplayType::Block;
+ auto isEnclosingItemBoundaryElement = [](const Element& element) {
+ auto* renderer = element.renderer();
+ if (!renderer)
+ return false;
+
+ if (element.hasTagName(HTMLNames::buttonTag))
+ return true;
+
+ if (element.hasTagName(HTMLNames::liTag) || element.hasTagName(HTMLNames::aTag)) {
+ auto displayType = renderer->style().display();
+ return displayType == DisplayType::Block || displayType == DisplayType::InlineBlock;
}
+
return false;
};
@@ -287,11 +295,12 @@
for (; !iterator.atEnd(); iterator.advance()) {
auto content = iterator.currentContent();
if (content.node) {
- if (enclosingParagraphElement && !enclosingParagraphElement->contains(content.node.get())) {
+ if (enclosingItemBoundaryElement && !enclosingItemBoundaryElement->contains(content.node.get())) {
if (!tokensInCurrentParagraph.isEmpty())
addItem(ManipulationItemData { startOfCurrentParagraph, endOfCurrentParagraph, nullptr, nullQName(), std::exchange(tokensInCurrentParagraph, { }) });
- enclosingParagraphElement = nullptr;
+ enclosingItemBoundaryElement = nullptr;
}
+
if (RefPtr<Element> currentElementAncestor = is<Element>(*content.node) ? downcast<Element>(content.node.get()) : content.node->parentOrShadowHostElement()) {
if (isInManipulatedElement(*currentElementAncestor))
return; // We can exit early here because scheduleObservartionUpdate calls this function on each paragraph separately.
@@ -311,8 +320,8 @@
}
}
}
- if (isEnclosingParagraphElement(currentElement))
- enclosingParagraphElement = ¤tElement;
+ if (!enclosingItemBoundaryElement && isEnclosingItemBoundaryElement(currentElement))
+ enclosingItemBoundaryElement = ¤tElement;
}
}
Modified: trunk/Tools/ChangeLog (260677 => 260678)
--- trunk/Tools/ChangeLog 2020-04-24 22:49:18 UTC (rev 260677)
+++ trunk/Tools/ChangeLog 2020-04-24 23:18:05 UTC (rev 260678)
@@ -1,3 +1,16 @@
+2020-04-24 Wenson Hsieh <[email protected]>
+
+ Make some more adjustments to TextManipulationController's paragraph boundary heuristic
+ https://bugs.webkit.org/show_bug.cgi?id=210993
+ <rdar://problem/61571299>
+
+ Reviewed by Tim Horton.
+
+ Add a new API test with buttons and links styled with `display: inline-block;`. Additionally, rebaseline an
+ existing API test that exercises inline-block list items.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:
+
2020-04-24 Chris Dumez <[email protected]>
[iOS] Always run WKTR's WKWebViews at foreground priority
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm (260677 => 260678)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm 2020-04-24 22:49:18 UTC (rev 260677)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm 2020-04-24 23:18:05 UTC (rev 260678)
@@ -616,6 +616,46 @@
EXPECT_WK_STREQ("More text", items[1].tokens[0].content);
}
+TEST(TextManipulation, StartTextManipulationTreatsInlineBlockLinksAndButtonsAsParagraphs)
+{
+ 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>"
+ " a {"
+ " display: inline-block;"
+ " border: 1px blue solid;"
+ " margin-left: 1em;"
+ " }"
+ " </style>"
+ "</head>"
+ "<body>"
+ " <button>One</button><button>Two</button>"
+ " <div><br></div>"
+ " <a href=''>Three</a><a href=''>Four</a>"
+ "</body>"];
+
+ done = false;
+ [webView _startTextManipulationsWithConfiguration:nil completion:^{
+ done = true;
+ }];
+ TestWebKitAPI::Util::run(&done);
+
+ NSArray<_WKTextManipulationItem *> *items = [delegate items];
+ EXPECT_EQ(items.count, 4UL);
+ EXPECT_EQ(items[0].tokens.count, 1UL);
+ EXPECT_EQ(items[1].tokens.count, 1UL);
+ EXPECT_EQ(items[2].tokens.count, 1UL);
+ EXPECT_EQ(items[3].tokens.count, 1UL);
+ EXPECT_WK_STREQ("One", items[0].tokens[0].content);
+ EXPECT_WK_STREQ("Two", items[1].tokens[0].content);
+ EXPECT_WK_STREQ("Three", items[2].tokens[0].content);
+ EXPECT_WK_STREQ("Four", items[3].tokens[0].content);
+}
+
struct Token {
NSString *identifier;
NSString *content;
@@ -1778,16 +1818,17 @@
TestWebKitAPI::Util::run(&done);
auto *items = [delegate items];
- EXPECT_EQ(items.count, 1UL);
- EXPECT_EQ(items[0].tokens.count, 2UL);
- EXPECT_STREQ("holle", items[0].tokens[0].content.UTF8String);
- EXPECT_STREQ("wdrlo", items[0].tokens[1].content.UTF8String);
+ EXPECT_EQ(items.count, 2UL);
+ EXPECT_EQ(items[0].tokens.count, 1UL);
+ EXPECT_EQ(items[1].tokens.count, 1UL);
+ EXPECT_WK_STREQ("holle", items[0].tokens[0].content);
+ EXPECT_WK_STREQ("wdrlo", items[1].tokens[0].content);
done = false;
- [webView _completeTextManipulationForItems:@[(_WKTextManipulationItem *)createItem(items[0].identifier, {
- { items[0].tokens[0].identifier, @"hello" },
- { items[0].tokens[1].identifier, @"world" },
- })] completion:^(NSArray<NSError *> *errors) {
+ [webView _completeTextManipulationForItems:@[
+ createItem(items[0].identifier, {{ items[0].tokens[0].identifier, @"hello" }}).autorelease(),
+ createItem(items[1].identifier, {{ items[1].tokens[0].identifier, @"world" }}).autorelease()
+ ] completion:^(NSArray<NSError *> *errors) {
EXPECT_EQ(errors, nil);
done = true;
}];
