Last Comment Bug 202930 - CSS text-decoration is computed correctly but not drawn on text inside TABLEs in Standards Compliance mode
: CSS text-decoration is computed correctly but not drawn on text inside TABLEs...
Status: RESOLVED DUPLICATE of bug 403526
(WG) needs better text proposal for C...
: css1, helpwanted, testcase
Product: Core
Classification: Components
Component: Layout: Tables (show other bugs)
: Trunk
: All All
: P3 normal with 4 votes (vote)
: Future
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
http://www.multrus.com/mozilla/test_s...
: 213867 248999 385943 412785 (view as bug list)
Depends on: 403524
Blocks:
  Show dependency treegraph
 
Reported: 2003-04-22 12:11 PDT by Markus Multrus
Modified: 2010-03-17 09:43 PDT (History)
19 users (show)
ispiked: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
The reporter's original testcase (810 bytes, text/html)
2003-04-22 15:07 PDT, Marco Perez
no flags Details
sort of a testcase... (791 bytes, text/html)
2003-04-22 20:05 PDT, Boris Zbarsky [:bz]
no flags Details
One possible patch (1.57 KB, patch)
2003-04-22 20:07 PDT, Boris Zbarsky [:bz]
no flags Details | Diff | Splinter Review
updated patch (1.56 KB, patch)
2007-04-26 05:46 PDT, Mathieu Fenniak
no flags Details | Diff | Splinter Review
complete minimized testcase for CSSWG (305 bytes, text/html)
2008-07-03 09:30 PDT, Zack Weinberg (:zwol)
no flags Details

Description Markus Multrus 2003-04-22 12:11:31 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4b) Gecko/20030422
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4b) Gecko/20030422

This occured somewhere before 1.3 final: If you now apply text-decoration:
line-through to the <tr>- tag, the text within the cells is not striked any
more. I don't know exactly, if this is the correct behaviour, but I can say that
e.g. Konquerer does strike the text

Reproducible: Always

Steps to Reproduce:
Comment 1 Markus Multrus 2003-04-22 12:41:24 PDT
added link to testcase, cause I'm not able to create an attachment with my
firewall settings... feel free to attach it to this bug :-)
Comment 2 Jo Hermans 2003-04-22 13:31:14 PDT
Shouldn't that be 
.striked td {text-decoration: line-through;}
to have a line in row 2 ?
Comment 3 Markus Multrus 2003-04-22 13:57:27 PDT
Is this correct that there is no inheritance for <td> from <tr>? Is <td> not a
block box from another block box <td>? Then sould be inheritance... This goes
out to the freaks... :) 
http://www.w3.org/TR/REC-CSS2/text.html#lining-striking-props
Comment 4 Marco Perez 2003-04-22 15:04:42 PDT
I can reproduce this on Win98.

According to the specs (http://www.w3.org/TR/REC-CSS2/text.html#q3) "This
property is not inherited, but descendant boxes of a block box should be
formatted with the same decoration...". Does this mean we have a valid bug here?

Checking the computed styles with DOM inspector reveals that text-decoration of
the cell in row 2 is set to "none", but this isn't defined in
resource:///res/html.css .
Specifying  td {text-decoration:inherit }  in the testcase may serve as a
workaround.
Comment 5 Marco Perez 2003-04-22 15:07:49 PDT
Created attachment 121326 [details]
The reporter's original testcase

Just in case the test URL becomes invalid someday ;-)
Comment 6 Boris Zbarsky [:bz] 2003-04-22 19:45:52 PDT
The spec sez:

  This property describes decorations that are added to the text of an element.
  If the property is specified for a block-level element, it affects all
  inline-level descendants of the element. If it is specified for (or affects)
  an inline-level element, it affects all boxes generated by the element. 

The use of "descendant" instead of "child" here makes me think that this should
indeed strike the text....

Confirming, over to tables; it sounds like tables override some Paint() method
and don't paint decorations correctly....
Comment 7 Boris Zbarsky [:bz] 2003-04-22 20:00:25 PDT
That said, it's not clear what setting text-decoration on a row should really
do.... Should the table display types be considered block-level for purposes of
text-decoration?  I mean things like <tr> and <thead>.... At the moment, they
are not.
Comment 8 Boris Zbarsky [:bz] 2003-04-22 20:05:52 PDT
Created attachment 121373 [details]
sort of a testcase...
Comment 9 Boris Zbarsky [:bz] 2003-04-22 20:07:18 PDT
Created attachment 121374 [details] [diff] [review]
One possible patch
Comment 10 Hixie (not reading bugmail) 2003-04-22 20:10:09 PDT
(For extra amusement, note that CSS3 Text disagrees with CSS2.1.)
Comment 11 Boris Zbarsky [:bz] 2003-04-22 20:16:04 PDT
So... reading CSS3 Text, a conforming impl would _not_ underline the text in:

<div style="text-decoration: underline">
  <p>Text</p>
</div>

correct?

If so, how likely are they to make it agree with CSS2?  ;)
Comment 12 Madhur Bhatia 2003-04-23 11:28:54 PDT
see this on winXP too 
Comment 13 Boris Zbarsky [:bz] 2003-04-23 17:00:41 PDT
ok, assigning to hixie to decide what the right behavior here is....
Comment 14 Hixie (not reading bugmail) 2003-04-23 17:18:02 PDT
The "right behaviour" is whichever one you want at the moment. The specs are
most likely to be changed at the end of their respective CR periods to match
what the majority of UAs do.
Comment 15 Boris Zbarsky [:bz] 2003-04-23 17:22:27 PDT
Well, I personally have no particular preference, and no real time to put into
thinking about it (as in, I have time to implement a behavior, but not to design
one). So any help would be much appreciated.
Comment 16 Hixie (not reading bugmail) 2003-04-23 17:27:42 PDT
I have no preference either. Either the root inline box picks up its text
decoration from its block only, or it crawls up to the root of its formatting
context adding text decorations that aren't yet set, or it crawls up to the
document's root element adding text decorations that aren't yet set. At the
moment I guess we are doing the second of those.
Comment 17 Boris Zbarsky [:bz] 2003-04-23 17:31:25 PDT
At the moment, it crawls up till it hits an "inline" frame ancestor.  Whatever
"inline" means.  Our current definition is "NOT block, table cell, list item, or
table".  All that patch does is change the defition of "inline" in this context
to also exclue "table-row, table-header-group, table-footer-group,
table-row-group, table-caption".
Comment 18 Hixie (not reading bugmail) 2003-04-23 17:34:40 PDT
Ok, that's two more possiblilities: Crawl up until you hit an inline; or crawl
up until you hit an inline or the top of the formatting context.
Comment 19 Boris Zbarsky [:bz] 2003-04-23 17:42:36 PDT
What's the actual definition of "top of formatting context"?  Is it just any
root or out of flow frame?
Comment 20 Hixie (not reading bugmail) 2003-04-23 17:47:29 PDT
Damn, I was hoping you wouldn't ask that. Usually if I use terms confidently
like that people just nod and smile. :-P

The term "formatting context" is one I made up. Roughly speaking, a new
formatting context is formed by table cells, absolutely positioned boxes,
non-visible 'overflow', floats, and the canvas.

(David has his own term for roughly the same thing, but they are not identical.)
Comment 21 Boris Zbarsky [:bz] 2003-04-23 17:57:06 PDT
Ian, I'm in math, remember?  Getting the definitions wrong is never good...  ;)

So it seems that as long as we're doing the behavior we're doing, not treating
<tr> and <tbody> (and <caption>) as blocks is silly... So we should either take
that patch or change the whole approach.
Comment 22 Hixie (not reading bugmail) 2003-04-23 19:53:32 PDT
Is there any reason we have to list everything but 'inline', rather than listing
just the things that should make us continue looking? I would guess that the
second list would be shorter and less prone to change. (I wouldn't want us to
have to change this code just because we add another display type.)
Comment 23 Boris Zbarsky [:bz] 2003-04-23 20:32:02 PDT
Well, our list of display types is at:

http://lxr.mozilla.org/seamonkey/source/layout/base/public/nsStyleConsts.h#311

so things suck no matter what.... ;)  What we really need are functions on
nsStyleDisplay like "isBlockRole()" and "isBlockContainer()" or whatever
dbaron's classifications were...
Comment 24 Hixie (not reading bugmail) 2003-04-23 21:12:50 PDT
Ugh.

Oh well. Pick a random subset of that list and go with it... ;-)
Comment 25 Boris Zbarsky [:bz] 2003-07-25 17:25:37 PDT
*** Bug 213867 has been marked as a duplicate of this bug. ***
Comment 26 Richard Brodie 2004-06-29 05:30:01 PDT
*** Bug 248999 has been marked as a duplicate of this bug. ***
Comment 27 Greg K. 2004-06-29 05:53:33 PDT
Sounds like text-decoration in general is not being drawn. Updating Summary.
(Note that since this affects captions as well it is not limited to TRs or TBODYs.)
Comment 28 Greg K. 2004-06-29 06:00:33 PDT
Whoops, bug 213867 is right, this only happens in Standards Compliance mode.
Comment 29 Greg K. 2004-07-04 08:02:36 PDT
Reproduced using Camino 0.8. Setting Hardware=All. Also, this probably belongs
on GFX, not Layout: Tables. (The style is being computed correctly, but not drawn.)
Comment 30 Boris Zbarsky [:bz] 2004-07-12 00:08:56 PDT
This is an issue in layout code, not GFX code.  I'm not likely to have time to
look at this any time soon, but I'll review patches....
Comment 31 Mathieu Fenniak 2007-04-26 05:46:49 PDT
Created attachment 262877 [details] [diff] [review]
updated patch

Attaching updated patch with current location of nsHTMLContainerFrame.cpp, but no code changes.

This patch does fix the problem, text-decoration filters through tables as one would expect from the CSS2 spec, and the behavior after the patch matches IE as well.

I've run this patch against the test-decoration test cases in bug 1777 and can confirm that there are no side-effects on any of the complex test cases there.  They all display identically after this patch, no changes.

Any chance this could move forward and be committed?
Comment 32 Boris Zbarsky [:bz] 2007-04-26 12:28:35 PDT
It's note clear that this patch gives the right behavior.  Until that happens...
Comment 33 Boris Zbarsky [:bz] 2007-04-26 12:29:54 PDT
Also note that now we do have IsBlockInside() and IsBlockOutside(), but that nether one includes internal table elements.

Also note that it's not clear how this should interact with inline-block and inline-table.
Comment 34 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2007-04-26 13:01:04 PDT
Note that, in general, the first step for getting a contributed patch incorporated is to request review on it.  This is done by setting the review flag (either when creating the attachment, or in the "Details" page for it) to "?" and putting in the email address of the reviewer (or a unique substring of their email or username).  In this case, Boris or I would probably be appropriate reviewers.

(In reply to comment #33)
> Also note that it's not clear how this should interact with inline-block and
> inline-table.

See also bug 371249.


What does the CSS2.1 spec say the correct behavior is?
Comment 35 Boris Zbarsky [:bz] 2007-04-26 13:04:49 PDT
> It's note clear that this patch gives the right behavior.

I meant "It's NOT clear".
Comment 36 Martijn Wargers [:mwargers] (not working for Mozilla) 2007-06-24 17:01:52 PDT
From: http://www.w3.org/TR/CSS21/text.html#q3
"
When specified on an inline element, it affects all the boxes generated by that element; for all other elements, 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. 
"

Definition of a block-level in-flow descendant is here:
http://www.w3.org/TR/CSS21/visuren.html#block-level
"
Block-level elements are those elements of the source document that are formatted visually as blocks (e.g., paragraphs). Several values of the 'display' property make an element block-level: 'block', 'list-item', and 'run-in' (part of the time; see run-in boxes), and 'table'. 
"
So it seems to me all table display values are considered block-level.
Comment 37 Adam Guthrie 2007-06-26 14:55:21 PDT
*** Bug 385943 has been marked as a duplicate of this bug. ***
Comment 38 Elmar Ludwig 2008-01-17 13:30:23 PST
*** Bug 412785 has been marked as a duplicate of this bug. ***
Comment 39 Zack Weinberg (:zwol) 2008-04-11 16:05:38 PDT
The current candidate patch for bug 403526 would also fix this bug in the manner requested by the original reporter.  As it's still undecided whether we actually want that behavior (though I am in favor of it) I am not going to do anything with this bug yet, other than add this note.
Comment 40 Zack Weinberg (:zwol) 2008-07-03 09:30:23 PDT
Created attachment 327978 [details]
complete minimized testcase for CSSWG

This is now a css2.1 open issue; see <http://csswg.inkedblade.net/spec/css2.1#issue-56>.  fantasai requested a complete test case.
Comment 41 Bernd 2009-07-18 10:09:50 PDT
the CSSWG needs a better proposal for the spec
Comment 42 Zack Weinberg (:zwol) 2010-03-17 09:43:25 PDT
The CSS2.1 issues list has not been updated, but 
http://lists.w3.org/Archives/Public/www-style/2009Nov/0237.html is the current proposal, I'm informed that it has been approved, and I am going to land the patch in bug 403526 that implements it.

*** This bug has been marked as a duplicate of bug 403526 ***

Note You need to log in before you can comment on or make changes to this bug.