Bug 88049 – Layout not updated after setting -webkit-line-clamp to none

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

<rdar://problem/11407365>

Changing -webkit-line-clamp dynamically from a number or percentage value to none doesn’t remove clamping. Patch forthcoming.

Created attachment 145222 [details]
Mark blocks for layout and clear truncation when line-clamp changes to none

[Abhishek Arya]

Comment on attachment 145222 [details]
Mark blocks for layout and clear truncation when line-clamp changes to none

View in context: https://bugs.webkit.org/attachment.cgi?id=145222&action=review

> Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp:153
> +void RenderDeprecatedFlexibleBox::styleWillChange(StyleDifference diff, const RenderStyle *newStyle)

* on the wrong side.

> Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp:987
> +void RenderDeprecatedFlexibleBox::clearLineClamp()

nit: Can this code be shared with http://code.google.com/codesearch#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp&exact_package=chromium&q=markPositionedObjectsForLayout&type=cs&l=883 to avoid duplication.

> Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp:995
> +            || (child->style()->height().isAuto() && child->isBlockFlow())) {

Do we want to use logicalHeight here ?

> Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp:996
> +            child->setChildNeedsLayout(true);

I think we just need to MarkOnlyThis in the second argument.

[Abhishek Arya]

Comment on attachment 145222 [details]
Mark blocks for layout and clear truncation when line-clamp changes to none

View in context: https://bugs.webkit.org/attachment.cgi?id=145222&action=review

> Source/WebCore/ChangeLog:14
> +        children for layout and clears trunaction from blocks.

typo 'trunacation'

Comment on attachment 145222 [details]
Mark blocks for layout and clear truncation when line-clamp changes to none

View in context: https://bugs.webkit.org/attachment.cgi?id=145222&action=review

Thanks for the review!

>> Source/WebCore/ChangeLog:14
>> +        children for layout and clears trunaction from blocks.
> 
> typo 'trunacation'

I am going to fix this.

>> Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp:153
>> +void RenderDeprecatedFlexibleBox::styleWillChange(StyleDifference diff, const RenderStyle *newStyle)
> 
> * on the wrong side.

Oops! I will fix this.

>> Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp:995
>> +            || (child->style()->height().isAuto() && child->isBlockFlow())) {
> 
> Do we want to use logicalHeight here ?

I don’t think RenderDeprecatedFlexibleBox was ever made writing-mode aware. More importantly, this code follows the logic in applyLineClamp().

>> Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp:996
>> +            child->setChildNeedsLayout(true);
> 
> I think we just need to MarkOnlyThis in the second argument.

MarkOnlyThis is only useful during layout, when you don’t want to re-dirty the ancestors. clearLineClamp() is not called during layout, so MarkOnlyThis is not necessary and using it would introduce risk of creating a subtree that needs layout and never gets it.

[Abhishek Arya]

Sounds good. Thanks for the patch.

Fixed in <http://trac.webkit.org/r119227>.