- Revision
- 264531
- Author
- [email protected]
- Date
- 2020-07-17 12:58:35 -0700 (Fri, 17 Jul 2020)
Log Message
Recommended article titles on csdn.net are clipped after translating to English
https://bugs.webkit.org/show_bug.cgi?id=214488
<rdar://problem/64531754>
Reviewed by Tim Horton.
Source/WebCore:
On csdn.net, the translation of the "荐" pill into English is "Recommended". This increase in the size of the
text node after translation breaks the layout of the page by clipping content below the pill, since the page
expects the element containing this text to have a fixed width and height.
Add a heuristic to detect and mitigate this exact scenario by applying `overflow: hidden;` onto containers
after translation, in the case where the container:
1. Did not have any visual overflow before translation.
2. Has visual overflow after translation.
3. Has both fixed width and fixed height.
4. Does not have any overflow clip.
5. Is not out-of-flow (i.e. fixed or absolute position).
While it does mean that the text inside the fixed-dimension container will be clipped, this will at least
prevent this overflow from breaking the rest of the page.
Test: TextManipulation.CompleteTextManipulationAddsOverflowHiddenToAvoidBreakingLayout
* editing/TextManipulationController.cpp:
(WebCore::TextManipulationController::completeManipulation):
(WebCore::TextManipulationController::replace):
While replacing content, keep track of nodes under which we're inserting content that didn't have any visual
overflow prior to replacement.
* editing/TextManipulationController.h:
Tools:
* TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:
(TestWebKitAPI::TEST):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (264530 => 264531)
--- trunk/Source/WebCore/ChangeLog 2020-07-17 19:47:04 UTC (rev 264530)
+++ trunk/Source/WebCore/ChangeLog 2020-07-17 19:58:35 UTC (rev 264531)
@@ -1,3 +1,38 @@
+2020-07-17 Wenson Hsieh <[email protected]>
+
+ Recommended article titles on csdn.net are clipped after translating to English
+ https://bugs.webkit.org/show_bug.cgi?id=214488
+ <rdar://problem/64531754>
+
+ Reviewed by Tim Horton.
+
+ On csdn.net, the translation of the "荐" pill into English is "Recommended". This increase in the size of the
+ text node after translation breaks the layout of the page by clipping content below the pill, since the page
+ expects the element containing this text to have a fixed width and height.
+
+ Add a heuristic to detect and mitigate this exact scenario by applying `overflow: hidden;` onto containers
+ after translation, in the case where the container:
+
+ 1. Did not have any visual overflow before translation.
+ 2. Has visual overflow after translation.
+ 3. Has both fixed width and fixed height.
+ 4. Does not have any overflow clip.
+ 5. Is not out-of-flow (i.e. fixed or absolute position).
+
+ While it does mean that the text inside the fixed-dimension container will be clipped, this will at least
+ prevent this overflow from breaking the rest of the page.
+
+ Test: TextManipulation.CompleteTextManipulationAddsOverflowHiddenToAvoidBreakingLayout
+
+ * editing/TextManipulationController.cpp:
+ (WebCore::TextManipulationController::completeManipulation):
+ (WebCore::TextManipulationController::replace):
+
+ While replacing content, keep track of nodes under which we're inserting content that didn't have any visual
+ overflow prior to replacement.
+
+ * editing/TextManipulationController.h:
+
2020-07-17 Simon Fraser <[email protected]>
Make ScrollbarControlSize an enum class
Modified: trunk/Source/WebCore/editing/TextManipulationController.cpp (264530 => 264531)
--- trunk/Source/WebCore/editing/TextManipulationController.cpp 2020-07-17 19:47:04 UTC (rev 264530)
+++ trunk/Source/WebCore/editing/TextManipulationController.cpp 2020-07-17 19:58:35 UTC (rev 264531)
@@ -604,6 +604,7 @@
auto TextManipulationController::completeManipulation(const Vector<WebCore::TextManipulationController::ManipulationItem>& completionItems) -> Vector<ManipulationFailure>
{
Vector<ManipulationFailure> failures;
+ HashSet<Ref<Node>> containersWithoutVisualOverflowBeforeReplacement;
for (unsigned i = 0; i < completionItems.size(); ++i) {
auto& itemToComplete = completionItems[i];
auto identifier = itemToComplete.identifier;
@@ -622,10 +623,30 @@
std::exchange(itemData, itemDataIterator->value);
m_items.remove(itemDataIterator);
- auto failureOrNullopt = replace(itemData, itemToComplete.tokens);
+ auto failureOrNullopt = replace(itemData, itemToComplete.tokens, containersWithoutVisualOverflowBeforeReplacement);
if (failureOrNullopt)
failures.append(ManipulationFailure { identifier, i, *failureOrNullopt });
}
+
+ if (!containersWithoutVisualOverflowBeforeReplacement.isEmpty()) {
+ if (m_document)
+ m_document->updateLayoutIgnorePendingStylesheets();
+
+ for (auto& container : containersWithoutVisualOverflowBeforeReplacement) {
+ if (!is<StyledElement>(container))
+ continue;
+
+ auto& element = downcast<StyledElement>(container.get());
+ auto* box = element.renderBox();
+ if (!box || !box->hasVisualOverflow())
+ continue;
+
+ auto* style = element.renderStyle();
+ if (style && style->width().isFixed() && style->height().isFixed() && !style->hasOutOfFlowPosition() && !style->hasClip())
+ element.setInlineStyleProperty(CSSPropertyOverflow, CSSValueHidden);
+ }
+ }
+
return failures;
}
@@ -678,7 +699,7 @@
insertions.append(NodeInsertion { lastTopDownPath.size() ? lastTopDownPath.last().second.ptr() : nullptr, *currentNode });
}
-auto TextManipulationController::replace(const ManipulationItemData& item, const Vector<ManipulationToken>& replacementTokens) -> Optional<ManipulationFailureType>
+auto TextManipulationController::replace(const ManipulationItemData& item, const Vector<ManipulationToken>& replacementTokens, HashSet<Ref<Node>>& containersWithoutVisualOverflowBeforeReplacement) -> Optional<ManipulationFailureType>
{
if (item.start.isOrphan() || item.end.isOrphan())
return ManipulationFailureType::ContentChanged;
@@ -824,12 +845,19 @@
node->remove();
for (auto& insertion : insertions) {
+ auto parentContainer = insertion.parentIfDifferentFromCommonAncestor;
if (!insertion.parentIfDifferentFromCommonAncestor) {
- insertionPoint.containerNode()->insertBefore(insertion.child, insertionPoint.computeNodeAfterPosition());
+ parentContainer = insertionPoint.containerNode();
+ parentContainer->insertBefore(insertion.child, insertionPoint.computeNodeAfterPosition());
insertionPoint = positionInParentAfterNode(insertion.child.ptr());
} else
insertion.parentIfDifferentFromCommonAncestor->appendChild(insertion.child);
+ if (auto* box = parentContainer->renderBox()) {
+ if (!box->hasVisualOverflow())
+ containersWithoutVisualOverflowBeforeReplacement.add(*parentContainer);
+ }
+
if (insertion.isChildManipulated == IsNodeManipulated::Yes)
m_manipulatedNodes.add(insertion.child.ptr());
}
Modified: trunk/Source/WebCore/editing/TextManipulationController.h (264530 => 264531)
--- trunk/Source/WebCore/editing/TextManipulationController.h 2020-07-17 19:47:04 UTC (rev 264530)
+++ trunk/Source/WebCore/editing/TextManipulationController.h 2020-07-17 19:58:35 UTC (rev 264531)
@@ -176,7 +176,7 @@
using NodeEntry = std::pair<Ref<Node>, Ref<Node>>;
Vector<Ref<Node>> getPath(Node*, Node*);
void updateInsertions(Vector<NodeEntry>&, const Vector<Ref<Node>>&, Node*, HashSet<Ref<Node>>&, Vector<NodeInsertion>&);
- Optional<ManipulationFailureType> replace(const ManipulationItemData&, const Vector<ManipulationToken>&);
+ Optional<ManipulationFailureType> replace(const ManipulationItemData&, const Vector<ManipulationToken>&, HashSet<Ref<Node>>& containersWithoutVisualOverflowBeforeReplacement);
WeakPtr<Document> m_document;
WeakHashSet<Element> m_elementsWithNewRenderer;
Modified: trunk/Tools/ChangeLog (264530 => 264531)
--- trunk/Tools/ChangeLog 2020-07-17 19:47:04 UTC (rev 264530)
+++ trunk/Tools/ChangeLog 2020-07-17 19:58:35 UTC (rev 264531)
@@ -1,3 +1,14 @@
+2020-07-17 Wenson Hsieh <[email protected]>
+
+ Recommended article titles on csdn.net are clipped after translating to English
+ https://bugs.webkit.org/show_bug.cgi?id=214488
+ <rdar://problem/64531754>
+
+ Reviewed by Tim Horton.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:
+ (TestWebKitAPI::TEST):
+
2020-07-17 Aakash Jain <[email protected]>
Delete code for old flakiness dashboard
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm (264530 => 264531)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm 2020-07-17 19:47:04 UTC (rev 264530)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm 2020-07-17 19:58:35 UTC (rev 264531)
@@ -2830,6 +2830,65 @@
EXPECT_WK_STREQ("end", items[2].tokens[0].content);
}
+TEST(TextManipulation, CompleteTextManipulationAddsOverflowHiddenToAvoidBreakingLayout)
+{
+ 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>"
+ "<head>"
+ " <style>"
+ " span {"
+ " display: inline-block;"
+ " width: 28px;"
+ " height: 40px;"
+ " margin: 0;"
+ " word-break: break-all;"
+ " font-size: 32px;"
+ " }"
+ " a {"
+ " display: block;"
+ " width: 100px;"
+ " height: 100px;"
+ " overflow: hidden;"
+ " }"
+ " </style>"
+ "</head>"
+ "<body>"
+ " <a>"
+ " <span>A</span> Hello world"
+ " </a>"
+ "</body>"
+ "</html>"];
+
+ done = false;
+ [webView _startTextManipulationsWithConfiguration:nil completion:^{
+ done = true;
+ }];
+ TestWebKitAPI::Util::run(&done);
+
+ auto items = [delegate items];
+ EXPECT_EQ(items.count, 2UL);
+ EXPECT_EQ(items[0].tokens.count, 1UL);
+ EXPECT_WK_STREQ("A", items[0].tokens[0].content);
+ EXPECT_EQ(items[1].tokens.count, 1UL);
+ EXPECT_WK_STREQ(" Hello world", items[1].tokens[0].content);
+
+ done = false;
+ [webView _completeTextManipulationForItems:@[
+ createItem(items[0].identifier, {{ items[0].tokens[0].identifier, @"This is a long translation" }}).get(),
+ createItem(items[1].identifier, {{ items[1].tokens[0].identifier, @" Foo bar" }}).get()
+ ] completion:^(NSArray<NSError *> *errors) {
+ EXPECT_EQ(errors, nil);
+ done = true;
+ }];
+ TestWebKitAPI::Util::run(&done);
+
+ EXPECT_WK_STREQ("This is a long translation Foo bar", [webView stringByEvaluatingJavaScript:@"document.querySelector('a').textContent.trim()"]);
+ EXPECT_WK_STREQ("hidden", [webView stringByEvaluatingJavaScript:@"getComputedStyle(document.querySelector('span')).overflow"]);
+}
+
TEST(TextManipulation, TextManipulationTokenDebugDescription)
{
auto token = adoptNS([[_WKTextManipulationToken alloc] init]);
