Closed Bug 229247 Opened 16 years ago Closed 10 years ago

Nav4 compatibility code executed even in standards mode

Categories

(Core :: Layout: Tables, defect)

x86
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: bernd_mozilla, Assigned: bernd_mozilla)

References

()

Details

Attachments

(1 file, 1 obsolete file)

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.
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;
   } 
removing the code does *not* break the layout regression tests
Summary: Nav4 compatibilty code executed even in standards mode → Nav4 compatibility code executed even in standards mode
I'd say remove that code, myself....
Assignee: layout.tables → nobody
QA Contact: layout.tables
Attached patch patchSplinter Review
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
Assignee: nobody → bernd_mozilla
Status: NEW → ASSIGNED
Attachment #396953 - Flags: review?(bzbarsky)
Comment on attachment 396953 [details] [diff] [review]
patch

r=bzbarsky
Attachment #396953 - Flags: review?(bzbarsky) → review+
http://hg.mozilla.org/mozilla-central/rev/a4b34d3053c5
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
This bug's patch removed the only use of the local variable 'availWidth', leaving it unused.  Attached patch nixes the variable.
Attachment #433885 - Flags: review?
Attachment #433885 - Flags: review? → review?(bernd_mozilla)
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.
Attachment #433885 - Attachment description: followup: remove unused variable 'availWidth' → followup: remove unused variable 'availWidth' (nevermind, this is bug 552246)
Attachment #433885 - Attachment is obsolete: true
Attachment #433885 - Flags: review?(bernd_mozilla)
Depends on: 552246
You need to log in before you can comment on or make changes to this bug.