- Revision
- 125635
- Author
- o...@chromium.org
- Date
- 2012-08-14 19:14:23 -0700 (Tue, 14 Aug 2012)
Log Message
Fix access to m_markupBox in WebCore::EllipsisBox::paint
https://bugs.webkit.org/show_bug.cgi?id=91138
Reviewed by Abhishek Arya.
Source/WebCore:
EllipsisBox would hold on to m_markupBox, which would then get destroyed during
the followup layoutIfNeeded in layoutVerticalBox. Instead, have EllipsisBox
dynamically grab to pointer to the markup box during paint since there's no
straightforward way to notify the EllipsisBox that the markupBox has been destroyed
and/or point it at the new markupBox.
Test: fast/overflow/line-clamp-and-columns.html
* rendering/EllipsisBox.cpp:
(WebCore::EllipsisBox::paint):
(WebCore):
(WebCore::EllipsisBox::paintMarkupBox):
* rendering/EllipsisBox.h:
(WebCore::EllipsisBox::EllipsisBox):
Just store a boolean that we have a markup box that needs painting.
* rendering/RenderDeprecatedFlexibleBox.cpp:
(WebCore::RenderDeprecatedFlexibleBox::applyLineClamp):
Clearing the override size right after setting it was incorrect because
there are cases where we'll do a followup layout in layoutVerticalBox, at which
point we'll still need the override size.
(WebCore::RenderDeprecatedFlexibleBox::clearLineClamp):
Clear the override size here to handle cases where line clamp is removed since
we don't call applyLineClamp in those cases.
LayoutTests:
* fast/overflow/line-clamp-and-columns-expected.html: Added.
* fast/overflow/line-clamp-and-columns.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (125634 => 125635)
--- trunk/LayoutTests/ChangeLog 2012-08-15 02:08:02 UTC (rev 125634)
+++ trunk/LayoutTests/ChangeLog 2012-08-15 02:14:23 UTC (rev 125635)
@@ -1,3 +1,13 @@
+2012-08-14 Ojan Vafai <o...@chromium.org>
+
+ Fix access to m_markupBox in WebCore::EllipsisBox::paint
+ https://bugs.webkit.org/show_bug.cgi?id=91138
+
+ Reviewed by Abhishek Arya.
+
+ * fast/overflow/line-clamp-and-columns-expected.html: Added.
+ * fast/overflow/line-clamp-and-columns.html: Added.
+
2012-08-14 Yoshifumi Inoue <yo...@chromium.org>
[Forms] Make input type "time" to use multiple field time input UI
Added: trunk/LayoutTests/fast/overflow/line-clamp-and-columns-expected.html (0 => 125635)
--- trunk/LayoutTests/fast/overflow/line-clamp-and-columns-expected.html (rev 0)
+++ trunk/LayoutTests/fast/overflow/line-clamp-and-columns-expected.html 2012-08-15 02:14:23 UTC (rev 125635)
@@ -0,0 +1,6 @@
+<!DOCTYPE html>
+<body style="width: 51px">
+<div id=flexbox style="-webkit-box-orient: vertical; -webkit-line-clamp: 1; display: -webkit-box; background-color: salmon; overflow:hidden">
+ <span>AAA </span><a href=""
+</div>
+</body>
Added: trunk/LayoutTests/fast/overflow/line-clamp-and-columns.html (0 => 125635)
--- trunk/LayoutTests/fast/overflow/line-clamp-and-columns.html (rev 0)
+++ trunk/LayoutTests/fast/overflow/line-clamp-and-columns.html 2012-08-15 02:14:23 UTC (rev 125635)
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<div id=flexbox style="-webkit-box-orient: vertical; -webkit-line-clamp: 1; display: -webkit-box; background-color: salmon; overflow:hidden">
+ <span>AAA </span><a href=""
+</div>
+<script>
+document.body.offsetTop;
+</script>
+<style>
+body {
+ -webkit-column-width: 50px;
+ -webkit-column-count: 12;
+}
+</style>
Modified: trunk/Source/WebCore/ChangeLog (125634 => 125635)
--- trunk/Source/WebCore/ChangeLog 2012-08-15 02:08:02 UTC (rev 125634)
+++ trunk/Source/WebCore/ChangeLog 2012-08-15 02:14:23 UTC (rev 125635)
@@ -1,3 +1,34 @@
+2012-08-14 Ojan Vafai <o...@chromium.org>
+
+ Fix access to m_markupBox in WebCore::EllipsisBox::paint
+ https://bugs.webkit.org/show_bug.cgi?id=91138
+
+ Reviewed by Abhishek Arya.
+
+ EllipsisBox would hold on to m_markupBox, which would then get destroyed during
+ the followup layoutIfNeeded in layoutVerticalBox. Instead, have EllipsisBox
+ dynamically grab to pointer to the markup box during paint since there's no
+ straightforward way to notify the EllipsisBox that the markupBox has been destroyed
+ and/or point it at the new markupBox.
+
+ Test: fast/overflow/line-clamp-and-columns.html
+
+ * rendering/EllipsisBox.cpp:
+ (WebCore::EllipsisBox::paint):
+ (WebCore):
+ (WebCore::EllipsisBox::paintMarkupBox):
+ * rendering/EllipsisBox.h:
+ (WebCore::EllipsisBox::EllipsisBox):
+ Just store a boolean that we have a markup box that needs painting.
+ * rendering/RenderDeprecatedFlexibleBox.cpp:
+ (WebCore::RenderDeprecatedFlexibleBox::applyLineClamp):
+ Clearing the override size right after setting it was incorrect because
+ there are cases where we'll do a followup layout in layoutVerticalBox, at which
+ point we'll still need the override size.
+ (WebCore::RenderDeprecatedFlexibleBox::clearLineClamp):
+ Clear the override size here to handle cases where line clamp is removed since
+ we don't call applyLineClamp in those cases.
+
2012-08-14 Yoshifumi Inoue <yo...@chromium.org>
[Forms] Make input type "time" to use multiple field time input UI
Modified: trunk/Source/WebCore/rendering/EllipsisBox.cpp (125634 => 125635)
--- trunk/Source/WebCore/rendering/EllipsisBox.cpp 2012-08-15 02:08:02 UTC (rev 125634)
+++ trunk/Source/WebCore/rendering/EllipsisBox.cpp 2012-08-15 02:14:23 UTC (rev 125635)
@@ -65,15 +65,31 @@
if (setShadow)
context->clearShadow();
- if (m_markupBox) {
- // Paint the markup box
- LayoutPoint adjustedPaintOffset = paintOffset;
- adjustedPaintOffset.move(x() + m_logicalWidth - m_markupBox->x(),
- y() + style->fontMetrics().ascent() - (m_markupBox->y() + m_markupBox->renderer()->style(isFirstLineStyle())->fontMetrics().ascent()));
- m_markupBox->paint(paintInfo, adjustedPaintOffset, lineTop, lineBottom);
- }
+ paintMarkupBox(paintInfo, paintOffset, lineTop, lineBottom, style);
}
+void EllipsisBox::paintMarkupBox(PaintInfo& paintInfo, const LayoutPoint& paintOffset, LayoutUnit lineTop, LayoutUnit lineBottom, RenderStyle* style)
+{
+ if (!m_shouldPaintMarkupBox || !m_renderer->isRenderBlock())
+ return;
+
+ RenderBlock* block = toRenderBlock(m_renderer);
+ RootInlineBox* lastLine = block->lineAtIndex(block->lineCount() - 1);
+ if (!lastLine)
+ return;
+
+ // 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.
+ InlineBox* anchorBox = lastLine->lastChild();
+ if (!anchorBox || !anchorBox->renderer()->style()->isLink())
+ return;
+
+ LayoutPoint adjustedPaintOffset = paintOffset;
+ adjustedPaintOffset.move(x() + m_logicalWidth - anchorBox->x(),
+ y() + style->fontMetrics().ascent() - (anchorBox->y() + anchorBox->renderer()->style(isFirstLineStyle())->fontMetrics().ascent()));
+ anchorBox->paint(paintInfo, adjustedPaintOffset, lineTop, lineBottom);
+}
+
IntRect EllipsisBox::selectionRect()
{
RenderStyle* style = m_renderer->style(isFirstLineStyle());
Modified: trunk/Source/WebCore/rendering/EllipsisBox.h (125634 => 125635)
--- trunk/Source/WebCore/rendering/EllipsisBox.h 2012-08-15 02:08:02 UTC (rev 125634)
+++ trunk/Source/WebCore/rendering/EllipsisBox.h 2012-08-15 02:14:23 UTC (rev 125635)
@@ -32,9 +32,9 @@
EllipsisBox(RenderObject* obj, const AtomicString& ellipsisStr, InlineFlowBox* parent,
int width, int height, int y, bool firstLine, bool isVertical, InlineBox* markupBox)
: InlineBox(obj, FloatPoint(0, y), width, firstLine, true, false, false, isVertical, 0, 0, parent)
+ , m_shouldPaintMarkupBox(markupBox)
, m_height(height)
, m_str(ellipsisStr)
- , m_markupBox(markupBox)
, m_selectionState(RenderObject::SelectionNone)
{
}
@@ -45,13 +45,14 @@
IntRect selectionRect();
private:
+ void paintMarkupBox(PaintInfo&, const LayoutPoint& paintOffset, LayoutUnit lineTop, LayoutUnit lineBottom, RenderStyle*);
virtual int height() const { return m_height; }
virtual RenderObject::SelectionState selectionState() { return m_selectionState; }
void paintSelection(GraphicsContext*, const LayoutPoint&, RenderStyle*, const Font&);
+ bool m_shouldPaintMarkupBox;
int m_height;
AtomicString m_str;
- InlineBox* m_markupBox;
RenderObject::SelectionState m_selectionState;
};
Modified: trunk/Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp (125634 => 125635)
--- trunk/Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp 2012-08-15 02:08:02 UTC (rev 125634)
+++ trunk/Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp 2012-08-15 02:14:23 UTC (rev 125635)
@@ -883,6 +883,7 @@
if (childDoesNotAffectWidthOrFlexing(child))
continue;
+ child->clearOverrideSize();
if (relayoutChildren || (child->isReplaced() && (child->style()->width().isPercent() || child->style()->height().isPercent()))
|| (child->style()->height().isAuto() && child->isBlockFlow())) {
child->setChildNeedsLayout(true, MarkOnlyThis);
@@ -921,7 +922,6 @@
child->setChildNeedsLayout(true, MarkOnlyThis);
child->setOverrideLogicalContentHeight(newHeight - child->borderAndPaddingHeight());
child->layoutIfNeeded();
- child->clearOverrideSize();
// FIXME: For now don't support RTL.
if (style()->direction() != LTR)
@@ -984,6 +984,7 @@
if (childDoesNotAffectWidthOrFlexing(child))
continue;
+ child->clearOverrideSize();
if ((child->isReplaced() && (child->style()->width().isPercent() || child->style()->height().isPercent()))
|| (child->style()->height().isAuto() && child->isBlockFlow())) {
child->setChildNeedsLayout(true);
