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

VERIFIED FIXED

Status

()

P1
critical
VERIFIED FIXED
19 years ago
19 years ago

People

(Reporter: buster, Assigned: rods)

Tracking

Trunk
x86
Windows NT
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

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

(Reporter)

Description

19 years ago
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>
(Reporter)

Comment 1

19 years ago
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

Comment 2

19 years ago
Please let us know the fix and risk.
Whiteboard: [NEED INFO]
(Assignee)

Comment 3

19 years ago
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
(Assignee)

Comment 4

19 years ago
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;
>   }
(Reporter)

Comment 5

19 years ago
r=buster.  agreed, very low risk, new code can only improve the situation.
(Assignee)

Comment 6

19 years ago
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

Comment 7

19 years ago
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.

Comment 8

19 years ago
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

Comment 9

19 years ago
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
(Assignee)

Comment 10

19 years ago
fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED

Comment 11

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