407243 - Table layout differs radically between FF2 and trunk

Status: RESOLVED WONTFIX

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

Description

[Duncan Loveday]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007120505 Minefield/3.0b2pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007120505 Minefield/3.0b2pre

In the attached test case, the layout of the table is very different on the current trunk as compared to Firefox 2.0.0.11 (and IE7).

The table is defined in a stupid way, having COLSPANs that don't add up to the same value for each row but nonetheless, consistent behaviour would produce fewer surprises.

Reproducible: Always

Steps to Reproduce:
1. Open the attached in FF 2.0.0.11 and the latest trunk.
2.
3.
Actual Results:  
Table layouts differ

Expected Results:  
Ideally, table layouts should be consistent unless FF 2.0.0.11 is actually wrong.

This test case is a reduction of the admin console for BEA's weblogic server version 8.1 sp 4. This is a J2EE server with significant market share so this change WILL be noticed.

Comment 1

[Duncan Loveday]

Attachment: Test case

Comment 2

[Duncan Loveday]

Attachment: Second test case

Having delved deeper, the weblogic console serves different HTML for IE as compared to Firefox so scrub the earlier comment about IE. Attaching a test case that more fully recreates what weblogic serves up for both IE and FF.

With what is served up for IE, all of FF2, FF3 and IE are all consistent.

With what is served up for FF, FF2, FF3 and IE are all different. FF2 shows text on the left of the screen, IE in the middle and FF3 on the right.

Comment 3

[Damon Sicore (:damons)]

+'ing with P3.  dbaron, can you make sure the priority is right here?

Comment 4

[(no longer active)]

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b2pre) Gecko/2007120605 Minefield/3.0b2pre

Confirming (not sure if it's necessary, since it's marked as blocking1.9+...)

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: reduced testcase

Comment 6

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

It sounds like this might be another case where we've cleaned up our error-handling behaviour to be row-order-independent etc. Daniel, hope you can at least diagnose that far.

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: testcase 4 (both row-orders)

This is similar to the previous reduced testcase, but with two tables showing both row-orderings.

This testcase also includes a description of how various browsers handle it.

Comment 8

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

Not sure what to do about this one. Either we don't fix it, or we block on it.

Comment 9

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

I think we should keep this on the blocking list to at least better understand what the fix here would look like.  (I'm actually not sure what it would look like.)  With a better idea of what the underlying algorithmic difference is, we might decide to wontfix or maybe just postpone for a later release.

Comment 10

[Damon Sicore (:damons)]

Yeah, let's decide a priority here.  P1 or P2?  Assuming we're blocking at the moment.

Comment 11

[Daniel Holbert [:dholbert]]

Below is a short explanation of how we're distributing the colspanning cells' widths in trunk. (labeling the columns "0, 1, 2, 3")

  Step A. Distribute light-blue spanning-cell's SMALL pref width among cols 1,2,3.
    ** It gets distributed equally, because these cols all have zero width.  (But it doesn't take effect yet -- it's stored in mSpanMinCoord and mSpanPrefCoord, rather than mMinCoord and mPrefCoord. This is part of maintaining row & col-order independence.)

 Step B. Distribute orange spanning-cell's LARGE pref width among cols 0,1,2.
   * All 500px of width goes to col 0, since it's the only column with any pref-width, from its 'x'.  (Cols 1 and 2 currently have zero pref width, since the width given to them from the light-blue cell hasn't taken effect yet)

 Step C. We call AcumulateSpanIntrinsics.  Now, the intrinsic widths that have been distributed actually take effect.

Also -- AFAIK, steps A and B may actually occur in any order, depending on the circumstances.  The important thing is, the outcome is the same either way, because the width that they distribute doesn't take effect until *both* steps are completed.

Comment 12

[Daniel Holbert [:dholbert]]

Attachment: testcase 5 (both row-orders AND both col-orders)

Comment 13

[Daniel Holbert [:dholbert]]

testcase 5 shows that FF2 is NOT column-order-independent.  FF2 matches trunk on orderings (1,0) and (1,1), but not (0,0) nor (0,1).

It looks like Firefox 2 is essentially going through the process described in Comment 11, except the width updates take effect *immediately* rather than being delayed.

As a result, the teal cell ends up being wide when we process the light-blue spanning cell first, because then columns 1 and 2 have nonzero pref-width when we distribute the 500px.  
  End result ==> Light blue cell is wide in (0,x) orderings.

BUT when we reverse the column-order, that apparently makes us process the spanning-cells in the reverse order, and then columns 1 and 2 have ZERO pref-width when we distribute the 500px.
  End result ==> Light blue cell is SKINNY (the same as in FF3) in the (0,1) orderings

So, it looks like the "regression" here is that we've made a change to more robust, more consistent behavior.

I'd suggest WONTFIXing this. (with a reftest to define the current behavior)  dbaron, do you agree?

Comment 14

[Daniel Holbert [:dholbert]]

(In reply to comment #13)
> As a result, the teal cell ends up being wide when we process the light-blue
> spanning cell first, because then columns 1 and 2 have nonzero pref-width when
> we distribute the 500px.  
>   End result ==> Light blue cell is wide in (0,x) orderings.

Sorry -- in the first line there, I meant "the light-blue cell ends up being wider", not "the teal cell ends up being wide"

Comment 15

[Daniel Holbert [:dholbert]]

Attachment: reftest [checked in]

Here's a reftest based on testcase 5 (attachment 307605 [details])

Comment 16

[Daniel Holbert [:dholbert]]

(In reply to comment #13)
> So, it looks like the "regression" here is that we've made a change to more
> robust, more consistent behavior.
> 
> I'd suggest WONTFIXing this. (with a reftest to define the current behavior) 
> dbaron, do you agree?
> 

Closing this as WONTFIX, per comment 13 -- dbaron, let me know if you disagree.

Comment 17

[Daniel Holbert [:dholbert]]

reftest (attachment 307616 [details] [diff] [review]) checked in, defining current behavior.

RCS file: /cvsroot/mozilla/layout/reftests/bugs/407243-1-ref.html,v
done
Checking in 407243-1-ref.html;
/cvsroot/mozilla/layout/reftests/bugs/407243-1-ref.html,v  <--  407243-1-ref.html
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/layout/reftests/bugs/407243-1.html,v
done
Checking in 407243-1.html;
/cvsroot/mozilla/layout/reftests/bugs/407243-1.html,v  <--  407243-1.html
initial revision: 1.1
done
Checking in reftest.list;
/cvsroot/mozilla/layout/reftests/bugs/reftest.list,v  <--  reftest.list
new revision: 1.388; previous revision: 1.387
done