269156 - Table cell width ignored when second table cell specifies larger width

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[Martijn Wargers (dead)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a5) Gecko/20041107 Firefox/0.9.1+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a5) Gecko/20041107 Firefox/0.9.1+

See upcoming testcase. 
That testcase shows, what I think, what is the initial cause of the overlap of
the text and some advertisement pictures on that page.

Basically, the problem is that when a table cell in a row below has a larger
width set as the table cell above (in a different row), the entire width of
these table cells get ignored.

Reproducible: Always
Steps to Reproduce:
1.
2.
3.

Comment 1

[Martijn Wargers (dead)]

Attachment: Testcase

The first table shows the problem. It becomes much wider than it should. In IE6
and Opera7.5 it gets the correct width (149px).

Comment 2

[Martijn Wargers (dead)]

I can even see the bug in Mozilla1.0, so this is not a (recent) regression.

Comment 3

[Bernd]

The problem is at
http://lxr.mozilla.org/seamonkey/source/layout/html/table/src/BasicTableLayoutStrategy.cpp#1070
In quirks mode  we see that the first cell has a larger desired (content driven)
width than the fixed width specified on the second table cell.

Comment 4

[Bernd]

Martijn, this bugs demonstrates that we are not implementing a quirk correctly.
The best thing would be of course to remove the quirk. In order to proceed with
this we/I  would need a more extended testcase. The code mentions that this not
only a NN, but also a IE quirk. 

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/layout/html/table/src&command=DIFF_FRAMESET&file=BasicTableLayoutStrategy.cpp&rev2=3.134&rev1=3.133
while it was originally only a navquirk maybe Chris only renamed it when we
started to use the quirk for emulating IE. The code was introduced to fix bug 14159.

What means more extended..

If one removes the first width spec then we render the testcase as NN4.x.
It is known that IE very carefully implemented all the bugs that NN had, (theire
quirks rendering) so the question is, did they completely ignore this flaw or
did they only implement one part of the flaw.
A  test matrix is needed, where the fixed width can be a) not set, b) smaller
than the desired width,c) larger than the desired width and differ between the
two cells. Is there any case when IE dropes a fixed width specification?

Comment 5

[Erik Fabert]

Attachment: testcase

I hope this goes in the right direction. Short summary:

Different between NN 4.75 and Mozilla 1.8a5: Tests 6, 11
Different between IE6 and NN 4.75: Tests 9, 16
Different between Mozilla 1.8a5 and IE6: Tests 6, 9, 11, 16

So in tests 6 and 11 Mozilla's behaviour is not compatible with either of the
two others. In tests 9 and 16 Mozilla implements Netscape 4 quirks IE6 did not
copy.

Also interesting is the difference in line breaking. If the table cell's
specified width is smaller than its real width, its content is still broken at
the specified width in both IE6 and NN 4.75.

Comment 6

[Bernd]

Erik: so is my understanding correct that IE always honours the fixed width?

Comment 7

[Erik Fabert]

AFAICT, yes, as long as the cell's content can be broken into small enough 
pieces. <td width=30>XXXXXXXXXXXXXXXX</td> or <td width=30><img width=200  
src=""></td> would result in cells wider than 30px.

Comment 8

[Bernd]

That smallest width the cell content can be broken too is called in mozilla the
Maximum Element Width (short MEW). Use that abbreviation and people will soon
ask you for reviews ;-). If the MEW is larger than the fixed width does it then
go to the MEW width or does it act like no specified fixed width.
<td width="30">xxxxxxxxxxxxxxxxxxx yyyy</td> will the yyyy wrap?

Comment 9

[Martijn Wargers (dead)]

(In reply to comment #8)
> <td width="30">xxxxxxxxxxxxxxxxxxx yyyy</td> will the yyyy wrap?
Yes, the yyyy wraps in IE6.
This is the testcase I used:
http://martijn.heelveel.info/test/mozilla/tablewidth/testcase.htm

Comment 10

[Bernd]

Attachment: patch

the patch fixes also a couple of hidden variables

Comment 11

[Bernd]

this is not the complete picture:
http://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/bugs/bug14159-1.html

Comment 12

[Bernd]

Attachment: patch

Comment 13

[Bernd]

the only thing that blocks me here is that as shown in
http://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/bugs/bug14159-1.html
there is some scenario when the width spec is not honoured by IE if somebody
could investigate when this exactly happens it should be easy to modify the patch.

Comment 14

[Martijn Wargers (dead)]

Isn't that scenario specific enough, then?
I've made this testcase:
http://martijn.heelveel.info/test/mozilla/td/bug14159-2.html
IE6 renders it as this:
http://martijn.heelveel.info/test/mozilla/td/ie_output.gif

It seems to me that IE6 is sort of 'ignoring' the width set in the table cell in
the first row. When the content in the table cell in the second table row
exceeds the width, then the table cell gets stretched to fit with the content.

This is not the case when the set width is on the table cell in the second row, see:
http://martijn.heelveel.info/test/mozilla/td/bug14159-3.html
IE6 output:
http://martijn.heelveel.info/test/mozilla/td/ie_output_3.gif

Comment 15

[Erik Fabert]

I don't think the behaviour in that case is desired, just a bug where they have 
used a cell's maximum width (I believe that is "desired width" in Mozilla's 
terms) when they should have used the minimum width (MEW?).

I'll try to explain the algorithm (W = column width):
1. Mark W as not fixed.
2. Scan the table rows from first to last
3. When a cell with a fixed width is found, mark W as fixed
4. Now here is the problem: if W is marked as fixed
and a new cell's width is not fixed, W is set to:
 W=max(W, cell.maximumWidth)
 This should really be W=max(W, cell.minimumWidth)
 
I hope that was understandable, if it's not I could attach a function in 
JavaScript to illustrate it. I wonder whether it is really necessary the 
implement this quirk? Opera doesn't and seems to get by just fine, are real 
sites relying on this?

Another problem I found is this case:

<table border>
	<tr><td colspan=2>123 456 789 012 345
	<tr><td width=50>12<td width=50>123
</table>

When a cell has a colspan > 1 but no fixed width, IE and NN4 ignore any other 
rows' fixed width. HTH...

Comment 16

[Bernd]

Attachment: ie quirk

<rant>I feel really bad working on a quirk, I thought that Marc's <img><img>
hack will be the last.</rant>
So when looking at this testcase we perfectly mimic NN4.78. But IE did not
fully implement the quirk. Please not that IE does not change the behaviour in
strict mode. What IE does, is it simply resets the fixed width if the desired
width is larger than a previoulsy recorded fixed width. The reset does not
happen if the width is specified on the cell with large desired width. Opera
does not implement this quirk at all, but I dont think that they count for this
issue.

Comment 17

[Bernd]

Comment on attachment 166346 [details] [diff] [review]
patch 

David, this more pleya for advice, than a request for review. We face 3 choices
- do nothing keep compatibility to NN4.x and I will spread my charm in those
bugs to explain that the bugs suck
- remove the quirk at all, become Opera compatible in all cases, but become in
one case IE compatible and loose compatibility in another case (this patch).
The risk is that we will wrap text/images more frequently
- do the bad thing and half implement the IE quirk as good as we understand it,
but dont implement the cell width limit, where a cell will be larger than the
fixed width but the text will wrap at the fixed width.

Comment 18

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

So I fixed this bug on the reflow branch.  I basically hadn't been able to wrap my head around your question at the time you asked it, and by the time I ended up working on the same thing on the reflow branch I'd forgotten about it.  So sorry for answering so late in the game, but anyway...

The basic principle I followed on the reflow branch was to be as compatible with WinIE as possible while maintaining row-order invariance -- and generally (although not completely) while maintaining the row-order-invariance in favor of Mozilla's old behavior.  (WinIE6 is the only modern browser whose table layout code gives different results when the rows are reordered.)  In the cases where IE6 gave different results in different row orders, such as this one, I went with what I thought was the more sensible approach.

I think row-order-invariance has some value, especially for things like dynamic Web apps (which might offer sorting of tabular data), and I think that existing browsers showed enough variation on the testcases where WinIE was not row-order-invariant that we can get away with it.  (Especially since all of the other modern browsers, including Gecko pre-reflow-branch, are row-order-invariant.)

The testcases I used for the new BasicTableLayoutStrategy are at http://dbaron.org/css/test/2006/intrinsic-examples/ ; I really need to turn those into regression tests at some point.

Comment 19

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

Comment on attachment 166346 [details] [diff] [review]
patch 

Clearing review request for code that doesn't exist anymore; sorry I never got to this when it was useful to do so.