Closed Bug 15841 Opened 25 years ago Closed 25 years ago

SELECT has the first option selected when it should have none

Categories

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

Product:
Core
Component:
Layout: Form Controls
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: bratell, Assigned: pollmann)

References

(
URL
)

Details

Attachments

(3 files)

patch that seems to work
598 bytes, patch
Details | Diff | Splinter Review
test you should check
1.24 KB, text/html
Details
Quick verification test
131 bytes, text/html
Details 
When opening http://bugzilla.mozilla.org/query.cgi with a current CVS build
(1999-10-08) all SELECT fields with no default have the first option selected.
They shouldn't have.
Assignee: karnaze → rods
Reassigning to Rod
BUG EXISTS ON:
 * Linux viewer, M10 release
 * Linux viewer 1999-10-08-08-M11

WORKS CORRECTLY ON:
 * Linux viewer 1999-09-25-08-M11
 * Linux viewer 1999-10-03-08-M11
 * Linux viewer 1999-10-05-11-M11

Raising severity to blocker since this blocks dogfood by preventing use of
bugzilla.  Yell if I shouldn't be doing this.
See the changes at line 1320 in:

http://cvs-mirror.mozilla.org/webtools/bonsai/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/layout/html/forms/src&command=DIFF_FRAMESET&file=nsListControlFrame.cpp&rev1=1.69&rev2=1.70&root=/cvsroot

which seemed to introduce logic that should only be in the combobox (and was
added to the combobox to fix bug 9136 on 9/17/99 00:24) into the listbox, where
it should not be.

That is, I think the problem is currently what's at line 1337 of
layout/html/forms/src/nsListControlFrame.cpp .  I might even pull and test this
theory, but probably not.

cc:ing pollmann@netscape.com
Assignee: rods → pollmann
This is in your new code. The patch looks good but you should review and check
it in.
*** Bug 16052 has been marked as a duplicate of this bug. ***
Status: NEW → ASSIGNED
OS: Windows NT → All
Hardware: PC → All
Target Milestone: M11
Yep, this is my mistake.  I will check in the patch suggested by dbaron.  I'm
going to have to trace down why comboboxes with nothing initially selected
aren't getting 0 selected though, as it looks like this should be done in
nsComboboxControlFrame::InitTextStr(), but it isn't working.
Ah, I see.  In InitTextStr, it just checks to see if
nsListControlFrame::GetSelectedItem returns "" before defaulting selection to
item 0.  In nsListControlFrame::GetSelectedItem, if nothing is selected, the
first item on the list is returned instead of "".

Since an item may actually have text of "", it might be safer to actually call
GetSelectedIndex, see if that returns -1, then set selectedindex to 0.  Rod,
does this seem like a reasonable change to you?  I tried it in my local tree and
it works fine.  (In addition to dbaron's change, of course)
You should make sure the (text screen) output of the above test is sensible.
That is, it should probably be -1, -1, 0, 0, 0, both before and after the
display is changed.

(Right now, the output is 0,-1,0,0,0 before the display and 0,0,0,0,0 after it.
Back in the 8-16 build, it was -1,-1,0,0,0 and 0,-1,0,-1,-1 - that was before
you fixed bug 9136.)

BTW, in NN 4.x, a NON-MULTIPLE select with no 'selected' would automatically
have the first item selected.  However, this disagrees with HTML 4.0:
http://www.w3.org/TR/REC-html40/interact/forms.html#h-17.6.1
*** Bug 16084 has been marked as a duplicate of this bug. ***
Eric, yes to "it might be safer to actually call GetSelectedIndex, see if that
returns -1, then set selectedindex to 0"
David, with your above-mentioned fix, plus the patch I mention, this make the
output of your testcase 0,-1,0,0,0 both before and after.  The case you make for
not preselecting option zero (-1,-1,0,0,0) is great, in that it is in line with
the spec.

We're currently deviating from the spec because both Nav and IE will preselect
option 0 in a combo with nothing selected.  This seems like reasonable behaviour
from a UI perspective, and many existing web pages are designed around this
model.

Imagine a drop-down list box with option 0 being "Select color" followed by
"red", "green", "blue", none of which are preselected.  Pages like this
frequently have an onChange handler that takes the user to a new web page when
"red", "green", or "blue" is selected.

If we follow the spec, we don't by default preselect "Select color".  A user
drops down the combo then clicks on "Select color", and onchange will be sent
out.  This breaks some existing pages.

If we follow precedent, and ignore the spec, we preselect "Select color".  A
user drops down the combo and clicks on "Select color.  Since there was no
change in selection, no onchange is sent out, and the page works.

I'm certain that the morally correct thing to do here is not to preselect
"Select color", let the people with broken pages clean them up and put the
SELECTED attribute on their first option.  Maybe the best thing to do would be
to determine if we are in Quirks mode or not before preselecting the option.
I'll look into this, as a separate bug.
Also note that if we are to follow the spec and *not* preselect the first
option, we have another dilema on our hands.  What do we show as the text of the
drop down box when the page is first rendered?

If we show the first option, then when a form is submitted, a user would expect
the value of the first option to be submitted.  But since it is not selected, we
can't submit anything.

If we show an empty string, then cases like the "Select color" case that I
described above are going to be even more broken because the user won't even see
the string "Select color" when the page is rendered.
I didn't really want you to change that behavior - I just misunderstood your
comment at 11:48am.  Although the HTML spec does say so, I sometimes wonder how
much thought went into some parts of some specs.  (You could show a blank spot,
though, and leave it out of the form submission, for standard mode.)
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Good suggestion.  I'm opening up a new bug to track that.

I've checked in a fix for the rest of this bug.
To verify, visit the attachment I'm about to make.
The listbox should have no options selected when it is first rendered.
New bug is bug 16157

    
    

    
  
*** Bug 16185 has been marked as a duplicate of this bug. ***

    
  
Status: RESOLVED → VERIFIED

    
    

    
  
Verified using attached test case. 1999101508 build, NT.

    
    

    
  
Suggest that regression-testing for this bug be done before M11, M12...

This bug blocked dogfood use of bugzilla in at least 2 ways: it blocked use
of the bugzilla query page with M10, and caused (and is still causing)
invalid data to be entered with new bug reports if the reporter leaves
Platform, OS, and Component as they come up using M10 (details: bug 16719).

This is no more than a hassle for anyone who knows bugzilla and form
controls well, but for first time bug reporters, likely using a milestone
release for testing, it can make using the query page an exercise in
frustration (they won't be able to guess why it's not working, never
having seen it before) and making their bug report less useful (possibly
unuseable) by corrupting some of the data unless they are observant.

    
    

    
  
*** Bug 17210 has been marked as a duplicate of this bug. ***

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: