Wrong width calculation in table with fixed layout

• Component: Tables
• Status: RESOLVED FIXED

[Beth Dakin]

Created attachment 39782 [details]
Reduction

Safari does a different width calculation than Firefox or IE for a cell of a table that has table-layout:fixed. This leads to serious problems with our UIs in Safari. In the other browsers the width for such containers is done by taking the last parent container with a defined width - this seems not to be the case for Safari.                                 
                                                                           
See attached HTML

[Beth Dakin]

<rdar://problem/6925121>

[Beth Dakin]

Created attachment 39783 [details]
Patch

Comment on attachment 39783 [details]
Patch

> +    int maxPercentValue = m_table->style()->width().isPercent() ? m_table->style()->width().calcValue(m_table->view()->width()) : 0;

I don’t understand why you use the RenderView here. Will this do the right thing if the table’s container is wider than the RenderView?

[Beth Dakin]

I used the RenderView because I found all other options in my test cases to be too small. I didn't have a test where the table's container is wider than the view though. I will write a quick test and get back to you on that.

[Beth Dakin]

Dan and I talked about this a bunch on IRC. Let me summarize here. In my debugging, I discovered that in this case, we can set the maxPrefWidth to anything ridiculously large, and we will get the correct result. Basically, the number is meaningless, it just needs to be Big Enough so it doesn't prevent calculations later down the line. Logically speaking, it seems like we are looking for the first container of the fixed table that has a non auto width, and we want to use that as out arbitrarily-large  size. (I call this solution firstNonAutoContainer.) However, for some reason this solution was always a few pixels short for me. Very frustrating!! Here are our options as I see them:

1. Set maxPrefWidth to the value of the biggest container we have. 
2. Set it to some other constant BIG number. 
3. Add a bool or something that answers isMyMaxPrefWidthEvenMeaningful(). 
4. Figure out why firstNonAutoContainer is always a few pixels short. 
5. New approach

I am going to whip up 1, because it will be fast, and it is what I meant for this patch to be all along. However, Dan pointed out that going with the size of the view is overly simple. We can always refine this is we decide that one of the other options is much better.

[Beth Dakin]

Created attachment 39792 [details]
New Patch

[Beth Dakin]

Created attachment 39886 [details]
Newer Patch

Here's a new iteration of this patch that implements the same idea. Dan and I were looking into this more, and we saw a very very similar quirk in RenderBlock's prefWidth calculation. This iteration of the patch makes this fix similar to that quirk we found in RenderBlock. 

The similar quirk was implemented in http://trac.webkit.org/changeset/4316 Hyatt wrote that patch and probably has the most expertise in this area, so we think he should probably review.

[Dave Hyatt]

Comment on attachment 39886 [details]
Newer Patch

r=me

[Beth Dakin]

Fixed in revision 48647.