Bug 18611 - text-decoration should not be propagated to floating and absolutely positioned block-level decendats
Summary: text-decoration should not be propagated to floating and absolutely positione...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 525.x (Safari 3.1)
Hardware: All All
: P1 Major
Assignee: Kentaro Hara
URL: http://www.hixie.ch/tests/adhoc/css/t...
Keywords:
Depends on:
Blocks:
 
Reported: 2008-04-18 18:55 PDT by jasneet
Modified: 2012-02-22 23:50 PST (History)
14 users (show)

See Also:


Attachments
Patch (40.17 KB, patch)
2012-01-12 00:10 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff
Patch (38.43 KB, patch)
2012-01-12 03:47 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff
Patch (38.54 KB, patch)
2012-01-12 18:00 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff
rebased patch for commit (38.64 KB, patch)
2012-01-12 23:12 PST, Kentaro Hara
haraken: commit-queue-
Details | Formatted Diff | Diff
patch for commit (38.63 KB, patch)
2012-01-12 23:27 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description jasneet 2008-04-18 18:55:58 PDT
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
Comment 1 Robert Blaut 2008-06-13 02:37:13 PDT
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]
Comment 2 Andras Nemeseri 2009-02-13 16:18:17 PST
I have tested the latest nightly and the problem is the same with inline-block elements.
Comment 3 Xianzhu Wang 2010-09-15 02:15:03 PDT
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
Comment 4 Xianzhu Wang 2010-09-15 04:07:22 PDT
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.
Comment 5 Kentaro Hara 2012-01-11 23:53:12 PST
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
Comment 6 Kentaro Hara 2012-01-12 00:10:16 PST
Created attachment 122175 [details]
Patch
Comment 7 Kent Tamura 2012-01-12 00:57:15 PST
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?
Comment 8 WebKit Review Bot 2012-01-12 01:02:03 PST
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
Comment 9 Kentaro Hara 2012-01-12 03:47:57 PST
Created attachment 122207 [details]
Patch
Comment 10 Kentaro Hara 2012-01-12 03:48:44 PST
(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.
Comment 11 Darin Adler 2012-01-12 17:20:29 PST
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.
Comment 12 Kentaro Hara 2012-01-12 17:26:11 PST
(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...
Comment 13 Kentaro Hara 2012-01-12 18:00:20 PST
Created attachment 122352 [details]
Patch
Comment 14 Kentaro Hara 2012-01-12 18:01:01 PST
(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.
Comment 15 WebKit Review Bot 2012-01-12 23:01:37 PST
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
Comment 16 Kentaro Hara 2012-01-12 23:12:50 PST
Created attachment 122373 [details]
rebased patch for commit
Comment 17 Darin Adler 2012-01-12 23:18:07 PST
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?
Comment 18 Kentaro Hara 2012-01-12 23:27:45 PST
Created attachment 122377 [details]
patch for commit
Comment 19 Kentaro Hara 2012-01-12 23:29:32 PST
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:-)
Comment 20 WebKit Review Bot 2012-01-13 01:17:59 PST
Comment on attachment 122377 [details]
patch for commit

Clearing flags on attachment: 122377

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