Closed Bug 31072 Opened 25 years ago Closed 25 years ago

[regression] combo box not reporting real maxElementSize, badly messes up table layout on bugzilla

Categories

(Core :: Layout: Form Controls, defect, P1)

Product:
Core
Component:
Layout: Form Controls
Platform:
x86
Windows NT
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: buster, Assigned: rods)

References

(
URL
)

Details

(Whiteboard: [PDT+]fix is in my tree) 

Here is the reduced test case.  If you remove the "selected" attribute from the 
second option, it works fine.  It looks like the combobox uses the size of the 
option with the "selected" attribute as the maxElementSize, rather than the size 
of the largest option like it does if there is no "selected" attribute.

<HTML><BODY>
<FORM>
  <TABLE BORDER=1>
  <TR>
    <TD>
      <SELECT>
        <OPTION VALUE="Macintosh">Macintosh</option>
        <OPTION SELECTED VALUE="PC">PC</option>
      </SELECT>
    </TD>
    <TD>Version:</TD>
  </TR>
  </table>
</form></body></html>
rod hasn't had a chance to look at this yet, but scanning the code it looks like 
an easy fix.  This has the potential to effect many common pages.
Severity: normal → critical
Keywords: beta1
Priority: P3 → P1
Please let us know the fix and risk.
Whiteboard: [NEED INFO]
I don't get it. The reduced testcase looks and works fine. What is the problem? 
I just pulled tables, I'll pull all of layout and make sure I am up to date.
Status: NEW → ASSIGNED
Here is the diff that fixes it, very low risk:
S:\mozilla\layout\html\forms\src>cvs diff nsComboboxControlFrame.cpp
Index: nsComboboxControlFrame.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/html/forms/src/nsComboboxControlFrame.cpp,v
retrieving revision 1.110
diff -r1.110 nsComboboxControlFrame.cpp
1070a1071,1074
>   if (nsnull != aDesiredSize.maxElementSize) {
>     aDesiredSize.maxElementSize->width  = aDesiredSize.width;
>     aDesiredSize.maxElementSize->height = aDesiredSize.height;
>   }

r=buster.  agreed, very low risk, new code can only improve the situation.
I reviewed it one more time to make sure that reflowing the dropdown wouldn't 
cause us to pass back a bad maxElementSize in aDesiredSize. And it looks good 
everywhere because we never reflow the dropdown with aDesiredSize, we always use 
a temp variable.

This is good to go.
Whiteboard: [NEED INFO] → fix is in my tree
This must go in asap, because it looks like the max element size was not being 
set which could result in either some very small or very large abnormalities. 
And if it doesn't go in then I am going to get 20 or 30 new table bugs.

I'm marking this PDT+ (on behalf of the PDT team), on the grounds that we have 
reviewed and tested this extensively with the top 100 as well as mail.
Whiteboard: fix is in my tree → [PDT+]fix is in my tree
The big question is: When will it land?
Please update the status whiteboard with this info.

Rick: If you like the code, please give an approval and lets get this off the radar.

Thanks,
Jim

fixed.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Verified with 2000-03-10.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.