76331 - Cell with some invisible content considered empty

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect, P2)

Description

[Hanns-Dierk Bergler]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux 2.4.2-4GB i586; en-US; rv:0.8.1+)
Gecko/20010416
BuildID:    20010416

http://www.heise.de/tp/english/inhalt/te/7393/1.html

Reproducible: Always
Steps to Reproduce:
1. go to http://www.heise.de/tp/english/inhalt/te/7393/1.html
2. scroll down to table with white background beginning with "The safe-deposit
is a service...
3. watch the borders of table...are not drawn correctly

Comment 1

[Andreas M. "Clarence" Schneider]

Hmm. There is <img width=7 height=0 src="/tp/icons/eck_0.gif" alt=""> and they
seem to expect that we scale it to the cell height.

Comment 2

[Hanns-Dierk Bergler]

Nav4x and MSIE 5x render table correctly...

Comment 3

[Christian Mattar]

This is WONTFIX in my book...
If the author explicitly says height="0" then height="0" it is...

Comment 4

[Christian Mattar]

Whoops, I think the actual problem is that when the image without the height
isn't rendered, there's no content in the table cell so that its borders and
background isn't displayed...
CCing Hixie.

Should this cell count as empty or not?

Comment 5

[Andreas M. "Clarence" Schneider]

Yes, I was wrong. It is a 7x7 transparent GIF and the cell should inherit the
background of its table.

IMHO, this cell is not empty. Same problem: <td>&zwnj;</td> and <td>&zwj;</td>.
What should we do with <td><hr style="display:none"></td> ?

Currently not treated as empty: <td><hr style="visibility:hidden"></td> and
<td><br></td>.

This is a quirks mode only problem.

Reopening. New summary. OS -> all (I see this on Win NT). Related to bug 57498.

Comment 6

[Andreas M. "Clarence" Schneider]

Attachment: testcase

Comment 7

[Christian Mattar]

I don't see this as quirks-mode only.
We also need rules in strict for when cells count as empty and when they don't, 
e.g. for the empty-cells-property.

Take a look at http://www.w3.org/TR/REC-CSS2/tables.html#empty-cells

There it says that any entity except from CR, TAB and space count as content.
Any stuff which has 'visibility:hidden' does not.

I personally think that any stuff which has 'visibility:hidden' set counts as 
content, or else there wouldn't be any difference to 'display:none' in the case 
of table cells.
(The W3C-spec says I'm wrong, but WTF...)

Comment 8

[Andreas M. "Clarence" Schneider]

Yes, can affect strict mode too. Made another testcase. Weird results,
especially if you compare both.

Comment 9

[Andreas M. "Clarence" Schneider]

Attachment: testcase for strict mode

Comment 10

[Hanns-Dierk Bergler]

found another page with described problem:
http://www.it-universum.de/index1024x768.htm

Comment 11

[Christian Mattar]

Nope, that's not exactly the same problem.
On that page, the image which should fill the cell and thus force Mozilla to 
draw the background doesn't seem to exist.
If you reload the page, you'll see that the cells are drawn at first, but then 
disappear when Necko notifies Layout that the URL of the image doesn't exist. 
This is correct beahivour IMO.

Comment 12

[Hanns-Dierk Bergler]

O.K. you are right, the images are missing but why is that working on
Opera 5.x, Netscape 4.x, IE 4x,5x ???

Comment 13

[Christian Mattar]

This is really an edge-case. Is a cell empty when there's an image with an 
invalid source in it? Please note that when you add an "alt"-attribute other 
than "" to the missing image (like one should anyway), the background is 
displayed.
However, we need a solution for this case, at least in quirks.

Comment 14

[Hanns-Dierk Bergler]

is anyone working on that, at least for 0.9.2?

Comment 15

[karnaze (gone)]

Moving to m0.9.3.

Comment 16

[Hanns-Dierk Bergler]

please work on that, there are many tables out there looking ugly with
Mozilla/Netscape 6.1b1

Comment 17

[Christian Mattar]

Do you know of any URLs except the one on www.heise.de where this problem 
occurs? Finding some Top100 URLs would be the best way to convince Netscape to 
fix this bug.

Please don't change priority or add the nsbeta1+ keyword. Those are adjusted by 
the developer/Netscape PDT, and this bug is already nominated for nsbeta1 
consideration.

Comment 18

[Blake Ross]

Missed 0.9.3.

Comment 19

[karnaze (gone)]

reassigning to m0.9.6

Comment 20

[karnaze (gone)]

attinasi, it sounds like this would be fixed if unloaded images were not treated 
as being empty, but I can't remember the bug number. 

Comment 21

[Jesse Ruderman]

Also happens on http://www.csn.ul.ie/~caolan/wingdings/proposal/.  The images in
one column break and don't have alt text, so the table cells borders in that
column disappear.

Comment 22

[Bernd]

attinasi will probably not work on those bugs :-(

Comment 23

[Hanns-Dierk Bergler]

Using Mozilla 1.3:

This bug seems to be solved (since months..?..) :-) The table draws correctly.
I think we can close it.

Comment 24

[Christian Mattar]

The page displays correctly now, but only because the site has been fixed, not
because Mozilla's behaviour has changed.

Reopening 

Comment 25

[Boris Zbarsky [:bzbarsky]]

Ian, what does the spec actually say about all this gunk?

Comment 26

[Hixie (not reading bugmail)]

# Empty cells and cells with the 'visibility' property set to 'hidden' are 
# considered to have no visible content. Visible content includes " " and 
# other whitespace except ASCII CR ("\0D"), LF ("\0A"), tab ("\09"), and space 
# ("\20").

I think a child element should be considered visible content, whether it is
visible or not.

Comment 27

[Boris Zbarsky [:bzbarsky]]

Ian, that's one of the least clear passages I've seen in the spec.  What does
"empty cells" mean in that context?  Whatever they are, they are considered to
have no visible content, no matter what their content actually is.

Also, is that "I think" a "Ian thinks" or a "the WG thinks (and will clarify
before CSS2.1 exits CR"?

Comment 28

[Hixie (not reading bugmail)]

> Ian, that's one of the least clear passages I've seen in the spec.

Agreed.

> Also, is that "I think" a "Ian thinks" or a "the WG thinks (and will clarify
> before CSS2.1 exits CR"?

It's an "Ian thinks". I added this to the CSS2.1 CR issues list (issue 29).

Comment 29

[Boris Zbarsky [:bzbarsky]]

Excellent.  Thank you.

Comment 30

[Martijn Wargers (dead)]

So when looking at:
http://www.w3.org/TR/CSS21/tables.html#empty-cells
"Cells are empty unless they contain one or more of the following:

    * floating content (including empty elements),
    * in-flow content (including empty elements) other than whitespace that has been collapsed away by the 'white-space' property handling. "
When looking at the testcase, it seems to me that Mozilla is handling all those examples correctly (I'm not sure about the '<img src="http://www.mozilla.org/images/mozilla-banner.gif" height=0 width=600>' case)

Comment 31

[Bernd]

Attachment: patch

Comment 32

[Bernd]

Attachment: patch without tabs

Comment 33

[Bernd]

Attachment: patch

Comment 34

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+    if (nsGkAtoms::textFrame == frameType) {
+      if (innerFrame->GetRect().width != 0)
+        return PR_TRUE;

This isn't right, you could have font-size:0 non-whitespace text. Use nsTextFrame::HasNoncollapsedCharacters instead. (Might want to add that to your test.)

+      nsIFrame *outOfFlowFrame = nsLayoutUtils::GetFloatFromPlaceholder(innerFrame);

I'd call that "floatFrame".

That's kind of a weird definition for 'empty' --- I'm surprised a cell containing an empty span should be treated as non-empty --- but given that's the definition, I guess this patch is good other than the comments above. Thanks!

Comment 35

[Bernd]

Attachment: revised patch

> That's kind of a weird definition for 'empty'
AKA- Hixie
the include of nsTextFrame.h does not come without a price it pulls in nsBidiUtils.h (thats the makefile change) and triggers a VC++ warning

C:\Programme\Microsoft Visual Studio 9.0\VC\INCLUDE\xlocale(342) : warning C4530
: C++-Handler verwendet, aber Entladesemantik ist nicht aktiviert. Geben Sie /EH
sc an.

http://msdn.microsoft.com/en-us/library/2axwkyt4.aspx
I do not understand how this can happen

Comment 36

[Bernd]

timeless told me to ignore the warning

Comment 37

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 336537 [details] [diff] [review]
revised patch

+        -I$(srcdir)/../../intl/unicharutil/util \

Should be tabs to match the context

Comment 38

[Bernd]

Attachment: revised patch for checkin

Comment 39

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Maybe CellHasVisibleContent should be renamed to reflect what it does?  (Also, is it worth having a pointer to the section of the spec that defines what it does so people understand why it does what it does -- since it's a little odd to consider an empty block as making something non-empty.)

Comment 40

[Bernd]

I followed to the letter the spec

"In the separated borders model, this property controls the rendering of borders 
and backgrounds around cells that have no visible content."
                                          ^^^^^^^^^^^^^^^^
I agree that the definition of empty is not what one expect from the wording but I would not call a function 

CellHasVisibleContentAccordingToCSS21

I am not a native English speaker and will change this function to whatever name you or roc think is sensible. My ugly proposal is above.

Comment 41

[Bernd]

attachement 338430 got checked in

but I will keep the bug open till the naming is solved

Comment 42

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I guess it's ok as it is, then.

Comment 43

[Bernd]

I will add a reference to the corresponding CSS2.1 section with my next checkin

Comment 44

[Bernd]

done