Bug 18611 – text-decoration should not be propagated to floating and absolutely positioned block-level decendats

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[jasneet]

I Steps:
Go to 
http://www.hixie.ch/tests/adhoc/css/text/decoration/005.html

II Issue:
The text at right side should not be underlined.

III Conclusion:
issue with text-decoration with block children

IV Other browsers:
IE7: not ok
FF3: not ok
Opera9.24: ok

V Nightly tested: 32005

[Robert Blaut]

Confirmed as CSS 2.1 violation bug. CSS 2.1 says"

"[...] the decorations are propagated to an anonymous inline box that wraps all the in-flow inline children of the element, and to any block-level in-flow descendants. It is not, however, further propagated to floating and absolutely positioned descendants, nor to the contents of 'inline-table' and 'inline-block' descendants." [http://www.w3.org/TR/CSS21/text.html#propdef-text-decoration]

[Andras Nemeseri]

I have tested the latest nightly and the problem is the same with inline-block elements.

[Xianzhu Wang]

According to http://www.w3help.org/zh-cn/causes/RT3002, the statuses of this issue in different newest browsers are:

Firefox 3.6.3: mostly fixed
Chrome and Safari: fail
IE8: OK
Opera: OK

[Xianzhu Wang]

Verified that Chrome 6.0.472.55 and 7.0.517.5 can handle text-decoration propagation to inline-block and inline-table correctly, that is, text-decoration is not propagated to inline-block and inline-table.

Text-decoration propagation to floating and absolutely positioned descendants is still not fixed.

[Kentaro Hara]

The latest spec says "text decorations are not propagated to any out-of-flow descendants, nor to the contents of atomic inline-level descendants such as inline blocks and inline tables":
http://www.w3.org/TR/2011/WD-css3-text-20110901/#decoration

[Kentaro Hara]

Created attachment 122175 [details]
Patch

[Kent Tamura]

Comment on attachment 122175 [details]
Patch

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

> LayoutTests/ChangeLog:19
> +        * fast/css/text-decoration-in-floating-or-abspositioned-element.html: Added.
> +        * platform/chromium-linux/fast/css/text-decoration-in-floating-or-abspositioned-element-expected.png: Added.
> +        * platform/chromium-linux/fast/css/text-decoration-in-floating-or-abspositioned-element-expected.txt: Added.
> +        * platform/mac-snowleopard/fast/css/text-decoration-in-floating-or-abspositioned-element-expected.png: Added.
> +        * platform/mac-snowleopard/fast/css/text-decoration-in-floating-or-abspositioned-element-expected.txt: Added.

Can you convert it to a reftest?

[WebKit Review Bot]

Comment on attachment 122175 [details]
Patch

Attachment 122175 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11108759

New failing tests:
fast/repaint/focus-layers.html

[Kentaro Hara]

Created attachment 122207 [details]
Patch

[Kentaro Hara]

(In reply to comment #7)
> (From update of attachment 122175 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=122175&action=review
> 
> > LayoutTests/ChangeLog:19
> > +        * fast/css/text-decoration-in-floating-or-abspositioned-element.html: Added.
> > +        * platform/chromium-linux/fast/css/text-decoration-in-floating-or-abspositioned-element-expected.png: Added.
> > +        * platform/chromium-linux/fast/css/text-decoration-in-floating-or-abspositioned-element-expected.txt: Added.
> > +        * platform/mac-snowleopard/fast/css/text-decoration-in-floating-or-abspositioned-element-expected.png: Added.
> > +        * platform/mac-snowleopard/fast/css/text-decoration-in-floating-or-abspositioned-element-expected.txt: Added.
> 
> Can you convert it to a reftest?

Done.

[Darin Adler]

Comment on attachment 122207 [details]
Patch

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

I am not entirely comfortable saying r=me until we have the answer to the position: relative question.

> Source/WebCore/rendering/RenderObject.cpp:2491
> +        if (curr->isFloating() || curr->isPositioned())
> +            return;

Will this code change also change the behavior for relative-positioned descendants? Should it? I think we need a test case covering that either way.

What about fixed-position descendants? Seems good to add that to the test too.

[Kentaro Hara]

(In reply to comment #11)
> (From update of attachment 122207 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=122207&action=review
> 
> I am not entirely comfortable saying r=me until we have the answer to the position: relative question.
> 
> > Source/WebCore/rendering/RenderObject.cpp:2491
> > +        if (curr->isFloating() || curr->isPositioned())
> > +            return;
> 
> Will this code change also change the behavior for relative-positioned descendants? Should it? I think we need a test case covering that either way.
> 
> What about fixed-position descendants? Seems good to add that to the test too.

Thanks.

It does not affect the behavior of relative-positioned descendants. It just affects the behavior of floating, absolutely-positioned and fixed-positioned descendants. As commented in RenderObject.h, isPositioned() checks if it is an absolute or fixed positioning. isRelPositioned() checks if it is relative positioning.

Anyway, I'll add those test cases. The patch is coming...

[Kentaro Hara]

Created attachment 122352 [details]
Patch

[Kentaro Hara]

(In reply to comment #12)
> (In reply to comment #11)
> > (From update of attachment 122207 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=122207&action=review
> Anyway, I'll add those test cases. The patch is coming...

darin: Added.

[WebKit Review Bot]

Comment on attachment 122352 [details]
Patch

Rejecting attachment 122352 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: /mnt/git/webkit-commit-queue/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/11152451

[Kentaro Hara]

Created attachment 122373 [details]
rebased patch for commit

[Darin Adler]

Comment on attachment 122373 [details]
rebased patch for commit

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

> LayoutTests/fast/css/text-decoration-in-descendants.html:7
> +.fixed { position: static; right: 0; }

Shouldn’t this be position: fixed?

[Kentaro Hara]

Created attachment 122377 [details]
patch for commit

[Kentaro Hara]

Comment on attachment 122373 [details]
rebased patch for commit

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

>> LayoutTests/fast/css/text-decoration-in-descendants.html:7
>> +.fixed { position: static; right: 0; }
> 
> Shouldn’t this be position: fixed?

Opps, right. Fixed it and committed. Thanks for reviewing the patch, this helps us a lot:-)

[WebKit Review Bot]

Comment on attachment 122377 [details]
patch for commit

Clearing flags on attachment: 122377

Committed r104907: <http://trac.webkit.org/changeset/104907>