Diff
Modified: trunk/LayoutTests/ChangeLog (282085 => 282086)
--- trunk/LayoutTests/ChangeLog 2021-09-07 15:00:21 UTC (rev 282085)
+++ trunk/LayoutTests/ChangeLog 2021-09-07 15:30:28 UTC (rev 282086)
@@ -1,3 +1,13 @@
+2021-09-07 Antti Koivisto <an...@apple.com>
+
+ Remove -webkit-line-clamp behaviour where last link is placed after ellipsis box
+ https://bugs.webkit.org/show_bug.cgi?id=229985
+
+ Reviewed by Alan Bujtas.
+
+ * fast/flexbox/line-clamp-link-after-ellipsis-expected.txt: Removed.
+ * fast/flexbox/line-clamp-link-after-ellipsis.html: Removed.
+
2021-08-26 Sergio Villar Senin <svil...@igalia.com>
Interoperability issue in margin collapsing with overflow:hidden elements
Deleted: trunk/LayoutTests/fast/flexbox/line-clamp-link-after-ellipsis-expected.txt (282085 => 282086)
--- trunk/LayoutTests/fast/flexbox/line-clamp-link-after-ellipsis-expected.txt 2021-09-07 15:00:21 UTC (rev 282085)
+++ trunk/LayoutTests/fast/flexbox/line-clamp-link-after-ellipsis-expected.txt 2021-09-07 15:30:28 UTC (rev 282086)
@@ -1,12 +0,0 @@
-Test that links painted after the ellipsis of a flexbox using -webkit-line-clamp is hit-testable.
-
-On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
-
-
-PASS document.getElementsByTagName('a')[0].textContent is "clicked"
-PASS successfullyParsed is true
-
-TEST COMPLETE
-line one
-line twoclicked
-
Deleted: trunk/LayoutTests/fast/flexbox/line-clamp-link-after-ellipsis.html (282085 => 282086)
--- trunk/LayoutTests/fast/flexbox/line-clamp-link-after-ellipsis.html 2021-09-07 15:00:21 UTC (rev 282085)
+++ trunk/LayoutTests/fast/flexbox/line-clamp-link-after-ellipsis.html 2021-09-07 15:30:28 UTC (rev 282086)
@@ -1,40 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<script src=""
-<style>
-span#container {
- display: inline-block; /* clamp the width of the enclosing P to its contents */
-}
-
-div#flexbox {
- -webkit-box-orient: vertical;
- -webkit-line-clamp: 1;
- display: -webkit-box;
- overflow-y: hidden;
-}
-</style>
-</head>
-<body>
-<span id="container">
- <div id="flexbox">line one<br>line two<a id="link" href="" _onclick_="this.textContent='clicked'">not clicked</a></div>
-</span>
-<script>
-description('Test that links painted after the ellipsis of a flexbox using -webkit-line-clamp is hit-testable.');
-
-var divRect = document.getElementById("flexbox").getBoundingClientRect();
-var aRect = document.getElementById("link").getBoundingClientRect();
-var clickX = divRect.left + divRect.width - (aRect.width / 2);
-var clickY = divRect.top + divRect.height - (aRect.height / 2);
-
-if (window.eventSender) {
- eventSender.mouseMoveTo(clickX, clickY);
- eventSender.mouseDown();
- eventSender.mouseUp();
-}
-
-shouldBeEqualToString("document.getElementsByTagName('a')[0].textContent", "clicked");
-</script>
-<script src=""
-</body>
-</html>
\ No newline at end of file
Modified: trunk/LayoutTests/platform/ios/TestExpectations (282085 => 282086)
--- trunk/LayoutTests/platform/ios/TestExpectations 2021-09-07 15:00:21 UTC (rev 282085)
+++ trunk/LayoutTests/platform/ios/TestExpectations 2021-09-07 15:30:28 UTC (rev 282086)
@@ -564,7 +564,6 @@
fast/files/filereader-zip-bundle.html [ Skip ]
fast/files/null-origin-string.html [ Skip ]
fast/files/workers/worker-apply-blob-url-to-xhr.html [ Skip ]
-fast/flexbox/line-clamp-link-after-ellipsis.html [ Skip ]
fast/forms/25153.html [ Skip ]
fast/forms/disabled-mousedown-event.html [ Skip ]
fast/forms/disabled-search-input.html [ Skip ]
Modified: trunk/Source/WebCore/ChangeLog (282085 => 282086)
--- trunk/Source/WebCore/ChangeLog 2021-09-07 15:00:21 UTC (rev 282085)
+++ trunk/Source/WebCore/ChangeLog 2021-09-07 15:30:28 UTC (rev 282086)
@@ -1,3 +1,29 @@
+2021-09-07 Antti Koivisto <an...@apple.com>
+
+ Remove -webkit-line-clamp behaviour where last link is placed after ellipsis box
+ https://bugs.webkit.org/show_bug.cgi?id=229985
+
+ Reviewed by Alan Bujtas.
+
+ -webkit-line-clamp has a bizarre and random behavior where the last link in a paragraph gets
+ rendered after the ellipsis box. It was exists to support some long-dead UI. No other engine does this.
+ The implementation is buggy and only works at all under a narrow set of cicrumstances.
+
+ This patch removes the behavior.
+
+ * rendering/LegacyEllipsisBox.cpp:
+ (WebCore::LegacyEllipsisBox::LegacyEllipsisBox):
+ (WebCore::LegacyEllipsisBox::paint):
+ (WebCore::LegacyEllipsisBox::nodeAtPoint):
+ (WebCore::LegacyEllipsisBox::markupBox const): Deleted.
+ (WebCore::LegacyEllipsisBox::paintMarkupBox): Deleted.
+ * rendering/LegacyEllipsisBox.h:
+ * rendering/LegacyRootInlineBox.cpp:
+ (WebCore::LegacyRootInlineBox::placeEllipsis):
+ * rendering/LegacyRootInlineBox.h:
+ * rendering/RenderDeprecatedFlexibleBox.cpp:
+ (WebCore::RenderDeprecatedFlexibleBox::applyLineClamp):
+
2021-08-26 Sergio Villar Senin <svil...@igalia.com>
Interoperability issue in margin collapsing with overflow:hidden elements
Modified: trunk/Source/WebCore/rendering/LegacyEllipsisBox.cpp (282085 => 282086)
--- trunk/Source/WebCore/rendering/LegacyEllipsisBox.cpp 2021-09-07 15:00:21 UTC (rev 282085)
+++ trunk/Source/WebCore/rendering/LegacyEllipsisBox.cpp 2021-09-07 15:30:28 UTC (rev 282086)
@@ -34,9 +34,8 @@
WTF_MAKE_ISO_ALLOCATED_IMPL(LegacyEllipsisBox);
-LegacyEllipsisBox::LegacyEllipsisBox(RenderBlockFlow& renderer, const AtomString& ellipsisStr, LegacyInlineFlowBox* parent, int width, int height, int y, bool firstLine, bool isHorizontal, LegacyInlineBox* markupBox)
+LegacyEllipsisBox::LegacyEllipsisBox(RenderBlockFlow& renderer, const AtomString& ellipsisStr, LegacyInlineFlowBox* parent, int width, int height, int y, bool firstLine, bool isHorizontal)
: LegacyInlineElementBox(renderer, FloatPoint(0, y), width, firstLine, true, false, false, isHorizontal, 0, 0, parent)
- , m_shouldPaintMarkupBox(markupBox)
, m_height(height)
, m_str(ellipsisStr)
{
@@ -45,7 +44,7 @@
#endif
}
-void LegacyEllipsisBox::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset, LayoutUnit lineTop, LayoutUnit lineBottom)
+void LegacyEllipsisBox::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset, LayoutUnit, LayoutUnit)
{
GraphicsContext& context = paintInfo.context();
const RenderStyle& lineStyle = this->lineStyle();
@@ -78,40 +77,8 @@
if (setShadow)
context.clearShadow();
-
- paintMarkupBox(paintInfo, paintOffset, lineTop, lineBottom, lineStyle);
}
-LegacyInlineBox* LegacyEllipsisBox::markupBox() const
-{
- if (!m_shouldPaintMarkupBox)
- return 0;
-
- LegacyRootInlineBox* lastLine = blockFlow().lastRootBox();
- if (!lastLine)
- return 0;
-
- // If the last line-box on the last line of a block is a link, -webkit-line-clamp paints that box after the ellipsis.
- // It does not actually move the link.
- LegacyInlineBox* anchorBox = lastLine->lastChild();
- if (!anchorBox || !anchorBox->renderer().style().isLink())
- return 0;
-
- return anchorBox;
-}
-
-void LegacyEllipsisBox::paintMarkupBox(PaintInfo& paintInfo, const LayoutPoint& paintOffset, LayoutUnit lineTop, LayoutUnit lineBottom, const RenderStyle& style)
-{
- LegacyInlineBox* markupBox = this->markupBox();
- if (!markupBox)
- return;
-
- LayoutPoint adjustedPaintOffset = paintOffset;
- adjustedPaintOffset.move(x() + logicalWidth() - markupBox->x(),
- y() + style.fontMetrics().ascent() - (markupBox->y() + markupBox->lineStyle().fontMetrics().ascent()));
- markupBox->paint(paintInfo, adjustedPaintOffset, lineTop, lineBottom);
-}
-
IntRect LegacyEllipsisBox::selectionRect()
{
const RenderStyle& lineStyle = this->lineStyle();
@@ -145,21 +112,10 @@
context.fillRect(snapRectToDevicePixelsWithWritingDirection(selectionRect, renderer().document().deviceScaleFactor(), run.ltr()), c);
}
-bool LegacyEllipsisBox::nodeAtPoint(const HitTestRequest& request, HitTestResult& result, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset, LayoutUnit lineTop, LayoutUnit lineBottom, HitTestAction hitTestAction)
+bool LegacyEllipsisBox::nodeAtPoint(const HitTestRequest& request, HitTestResult& result, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset, LayoutUnit, LayoutUnit, HitTestAction)
{
LayoutPoint adjustedLocation = accumulatedOffset + LayoutPoint(topLeft());
- // Hit test the markup box.
- if (LegacyInlineBox* markupBox = this->markupBox()) {
- const RenderStyle& lineStyle = this->lineStyle();
- LayoutUnit mtx { adjustedLocation.x() + logicalWidth() - markupBox->x() };
- LayoutUnit mty { adjustedLocation.y() + lineStyle.fontMetrics().ascent() - (markupBox->y() + markupBox->lineStyle().fontMetrics().ascent()) };
- if (markupBox->nodeAtPoint(request, result, locationInContainer, LayoutPoint(mtx, mty), lineTop, lineBottom, hitTestAction)) {
- blockFlow().updateHitTestResult(result, locationInContainer.point() - LayoutSize(mtx, mty));
- return true;
- }
- }
-
auto boundsRect = LayoutRect { adjustedLocation, LayoutSize(LayoutUnit(logicalWidth()), m_height) };
if (visibleToHitTesting(request) && locationInContainer.intersects(boundsRect)) {
blockFlow().updateHitTestResult(result, locationInContainer.point() - toLayoutSize(adjustedLocation));
Modified: trunk/Source/WebCore/rendering/LegacyEllipsisBox.h (282085 => 282086)
--- trunk/Source/WebCore/rendering/LegacyEllipsisBox.h 2021-09-07 15:00:21 UTC (rev 282085)
+++ trunk/Source/WebCore/rendering/LegacyEllipsisBox.h 2021-09-07 15:30:28 UTC (rev 282086)
@@ -30,7 +30,7 @@
class LegacyEllipsisBox final : public LegacyInlineElementBox {
WTF_MAKE_ISO_ALLOCATED(LegacyEllipsisBox);
public:
- LegacyEllipsisBox(RenderBlockFlow&, const AtomString& ellipsisStr, LegacyInlineFlowBox* parent, int width, int height, int y, bool firstLine, bool isHorizontal, LegacyInlineBox* markupBox);
+ LegacyEllipsisBox(RenderBlockFlow&, const AtomString& ellipsisStr, LegacyInlineFlowBox* parent, int width, int height, int y, bool firstLine, bool isHorizontal);
void paint(PaintInfo&, const LayoutPoint&, LayoutUnit lineTop, LayoutUnit lineBottom) override;
bool nodeAtPoint(const HitTestRequest&, HitTestResult&, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset, LayoutUnit lineTop, LayoutUnit lineBottom, HitTestAction) final;
void setSelectionState(RenderObject::HighlightState s) { m_selectionState = s; }
@@ -39,13 +39,10 @@
RenderBlockFlow& blockFlow() const { return downcast<RenderBlockFlow>(LegacyInlineBox::renderer()); }
private:
- void paintMarkupBox(PaintInfo&, const LayoutPoint& paintOffset, LayoutUnit lineTop, LayoutUnit lineBottom, const RenderStyle&);
int height() const { return m_height; }
RenderObject::HighlightState selectionState() override { return m_selectionState; }
void paintSelection(GraphicsContext&, const LayoutPoint&, const RenderStyle&, const FontCascade&);
- LegacyInlineBox* markupBox() const;
- bool m_shouldPaintMarkupBox;
RenderObject::HighlightState m_selectionState { RenderObject::HighlightState::None };
int m_height;
AtomString m_str;
Modified: trunk/Source/WebCore/rendering/LegacyRootInlineBox.cpp (282085 => 282086)
--- trunk/Source/WebCore/rendering/LegacyRootInlineBox.cpp 2021-09-07 15:00:21 UTC (rev 282085)
+++ trunk/Source/WebCore/rendering/LegacyRootInlineBox.cpp 2021-09-07 15:30:28 UTC (rev 282086)
@@ -127,13 +127,13 @@
return LegacyInlineFlowBox::canAccommodateEllipsis(ltr, blockEdge, ellipsisWidth);
}
-float LegacyRootInlineBox::placeEllipsis(const AtomString& ellipsisStr, bool ltr, float blockLeftEdge, float blockRightEdge, float ellipsisWidth, LegacyInlineBox* markupBox)
+float LegacyRootInlineBox::placeEllipsis(const AtomString& ellipsisStr, bool ltr, float blockLeftEdge, float blockRightEdge, float ellipsisWidth)
{
if (!gEllipsisBoxMap)
gEllipsisBoxMap = new EllipsisBoxMap();
ASSERT(!hasEllipsisBox());
- auto* ellipsisBox = gEllipsisBoxMap->set(this, makeUnique<LegacyEllipsisBox>(blockFlow(), ellipsisStr, this, ellipsisWidth - (markupBox ? markupBox->logicalWidth() : 0), logicalHeight(), y(), !prevRootBox(), isHorizontal(), markupBox)).iterator->value.get();
+ auto* ellipsisBox = gEllipsisBoxMap->set(this, makeUnique<LegacyEllipsisBox>(blockFlow(), ellipsisStr, this, ellipsisWidth, logicalHeight(), y(), !prevRootBox(), isHorizontal())).iterator->value.get();
setHasEllipsisBox(true);
// FIXME: Do we need an RTL version of this?
if (ltr && (x() + logicalWidth() + ellipsisWidth) <= blockRightEdge) {
Modified: trunk/Source/WebCore/rendering/LegacyRootInlineBox.h (282085 => 282086)
--- trunk/Source/WebCore/rendering/LegacyRootInlineBox.h 2021-09-07 15:00:21 UTC (rev 282085)
+++ trunk/Source/WebCore/rendering/LegacyRootInlineBox.h 2021-09-07 15:30:28 UTC (rev 282086)
@@ -107,7 +107,7 @@
bool lineCanAccommodateEllipsis(bool ltr, int blockEdge, int lineBoxEdge, int ellipsisWidth);
// Return the truncatedWidth, the width of the truncated text + ellipsis.
- float placeEllipsis(const AtomString& ellipsisStr, bool ltr, float blockLeftEdge, float blockRightEdge, float ellipsisWidth, LegacyInlineBox* markupBox = nullptr);
+ float placeEllipsis(const AtomString& ellipsisStr, bool ltr, float blockLeftEdge, float blockRightEdge, float ellipsisWidth);
// Return the position of the LegacyEllipsisBox or -1.
float placeEllipsisBox(bool ltr, float blockLeftEdge, float blockRightEdge, float ellipsisWidth, float &truncatedWidth, bool& foundBox) final;
Modified: trunk/Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp (282085 => 282086)
--- trunk/Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp 2021-09-07 15:00:21 UTC (rev 282085)
+++ trunk/Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp 2021-09-07 15:30:28 UTC (rev 282086)
@@ -1055,22 +1055,11 @@
if (!lastVisibleLine || !lastVisibleLine->firstChild())
continue;
- const UChar ellipsisAndSpace[2] = { horizontalEllipsis, ' ' };
- static MainThreadNeverDestroyed<const AtomString> ellipsisAndSpaceStr(ellipsisAndSpace, 2);
static MainThreadNeverDestroyed<const AtomString> ellipsisStr(&horizontalEllipsis, 1);
const RenderStyle& lineStyle = numVisibleLines == 1 ? firstLineStyle() : style();
const FontCascade& font = lineStyle.fontCascade();
- // Get ellipsis width, and if the last child is an anchor, it will go after the ellipsis, so add in a space and the anchor width too
- LayoutUnit totalWidth;
- LegacyInlineBox* anchorBox = lastLine->lastChild();
- auto& lastVisibleRenderer = lastVisibleLine->firstChild()->renderer();
- if (anchorBox && anchorBox->renderer().style().isLink() && &lastVisibleRenderer != &anchorBox->renderer())
- totalWidth = anchorBox->logicalWidth() + font.width(constructTextRun(ellipsisAndSpace, 2, style()));
- else {
- anchorBox = nullptr;
- totalWidth = font.width(constructTextRun(&horizontalEllipsis, 1, style()));
- }
+ auto totalWidth = font.width(constructTextRun(&horizontalEllipsis, 1, style()));
// See if this width can be accommodated on the last visible line
RenderBlockFlow& destBlock = lastVisibleLine->blockFlow();
@@ -1091,7 +1080,7 @@
// Let the truncation code kick in.
// FIXME: the text alignment should be recomputed after the width changes due to truncation.
LayoutUnit blockLeftEdge = destBlock.logicalLeftOffsetForLine(LayoutUnit(lastVisibleLine->y()), DoNotIndentText);
- lastVisibleLine->placeEllipsis(anchorBox ? ellipsisAndSpaceStr : ellipsisStr, leftToRight, blockLeftEdge, blockRightEdge, totalWidth, anchorBox);
+ lastVisibleLine->placeEllipsis(ellipsisStr, leftToRight, blockLeftEdge, blockRightEdge, totalWidth);
destBlock.setHasMarkupTruncation(true);
}
}
