229247 - Nav4 compatibility code executed even in standards mode

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[Bernd]

When reading the patch for bug 101759, I found the following code.
http://lxr.mozilla.org/seamonkey/source/layout/html/table/src/nsTableRowFrame.cpp#679

It should be either removed at all or at least be escaped by a navquirks check.

Comment 1

[Bernd]

buster added in
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsTableRowFrame.cpp&branch=&root=/cvsroot&subdir=mozilla/layout/html/table/src&command=DIFF_FRAMESET&rev1=3.31&rev2=3.32

    // begin special Nav4 compatibility code                                    	
    if (0==cellWidth)                                                          
                                                                                
    {                                                                          
                                                                                
      cellWidth = aState.tableFrame->GetColumnWidth(cellColIndex);
    }                                                                           
    // end special Nav4 compatibility code 

then it changed with 
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsTableRowFrame.cpp&branch=&root=/cvsroot&subdir=mozilla/layout/html/table/src&command=DIFF_FRAMESET&rev1=3.67&rev2=3.68

to 

    cellWidth = availWidth;

then the code was copied over at

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsTableRowFrame.cpp&branch=&root=/cvsroot&subdir=mozilla/layout/html/table/src&command=DIFF_FRAMESET&rev1=3.187&rev2=3.188

and with karnazes checkin it became:

   if ((0 == aDesiredWidth) && (NS_UNCONSTRAINEDSIZE != aAvailWidth)) { //
special Nav4 compatibility code for the width
     aDesiredWidth = aAvailWidth;
   } 

Comment 2

[Bernd]

removing the code does *not* break the layout regression tests

Comment 3

[Boris Zbarsky [:bzbarsky]]

I'd say remove that code, myself....

Comment 4

[Bernd]

the code is still in the tree http://mxr.mozilla.org/seamonkey/source/layout/tables/nsTableRowFrame.cpp#657

Comment 5

[Bernd]

Attachment: patch

The code is no op, at least after the reflow branch landing. In the normal reflow we do what this code does not only for 0 desired width but for any width, the two other calls to this function ignore the width entirely they are focused on the height

Comment 6

[Boris Zbarsky [:bzbarsky]]

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

r=bzbarsky

Comment 7

[Bernd]

http://hg.mozilla.org/mozilla-central/rev/a4b34d3053c5

Comment 8

[Daniel Holbert [:dholbert]]

Attachment: followup: remove unused variable 'availWidth' (nevermind, this is bug 552246)

This bug's patch removed the only use of the local variable 'availWidth', leaving it unused.  Attached patch nixes the variable.

Comment 9

[Daniel Holbert [:dholbert]]

Comment on attachment 433885 [details] [diff] [review]
followup: remove unused variable 'availWidth' (nevermind, this is bug 552246)

looks like timeless already posted a followup patch to remove the unused variable in bug 552246. Obsoleting the version here.