Bug 37530 – No default selection for <select multiple> menu lists.

• Status: CLOSED
• Resolution: FIXED
• Component: WebCore Misc.

[Luiz Agostini]

No default selection for <select multiple> menu lists.

[Luiz Agostini]

Created attachment 53297 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 53297 [details]
Patch

This either needs a test case,or the ChangeLog needs to explain why one is not possible.  Seems a LayoutTest should be possible, certainly a manual-test is.

[Adele Peterson]

There are lots of examples of layout tests for <select multiple> that you can look at.  You'll also want to make sure to run all LayoutTests to see if this changes behavior in any of the existing tests.

[Luiz Agostini]

Created attachment 53368 [details]
patch 2

[Luiz Agostini]

This patch was motivated by the fact that usesMenuList() will return always true if NO_LISTBOX_RENDERING is enabled.
As NO_LISTBOX_RENDERING is a compile time flag and as it is enabled by default just for maemo5 LayoutTests would not be effective.
I added a manual test in latest patch.

[Luiz Agostini]

(In reply to comment #3)
> There are lots of examples of layout tests for <select multiple> that you can
> look at.  You'll also want to make sure to run all LayoutTests to see if this
> changes behavior in any of the existing tests.

I added the actual feature in previous patches that are already landed. They caused no regression.

[Antti Koivisto]

Comment on attachment 53368 [details]
patch 2

> This patch makes it possible for <select multiple> and <select size="n gt 1"> elements to have no selected option
> when rendered as menu lists.

I don't quite understand what is the problem and how this patch solves it. Could you explain this in the ChangeLog too? You say "makes it possible" but why, and how?

[Luiz Agostini]

(In reply to comment #7)
> (From update of attachment 53368 [details])
> > This patch makes it possible for <select multiple> and <select size="n gt 1"> elements to have no selected option
> > when rendered as menu lists.
> 
> I don't quite understand what is the problem and how this patch solves it.
> Could you explain this in the ChangeLog too? You say "makes it possible" but
> why, and how?

The target here is to have exactly the same behavior with and without NO_LISTBOX_RENDERING enabled. I mean, the very same <select> element in the very same html file must have the very same selection after loading the page with and without  NO_LISTBOX_RENDERING enabled.

The problem is that, for menu lists, if the selection is not indicated by the html file the first <option> will be selected after loading the page or reseting the form. Please consider bugzilla advanced search form: https://bugs.webkit.org/query.cgi?format=advanced . When the page is loaded many of its listboxes do not have a selected element (for example product, component, version). But when NO_LISTBOX_RENDERING is enabled the listboxes will became menu lists and then they will have an item selected after the page is loaded. That is the behavior difference that this patch corrects.

As when NO_LISTBOX_RENDERING is enabled usesMenuList() always returns true usesMenuList() cannot be used to decide about initial selection of the elements. This patch replaces (usesMenuLists()) by (!multiple && size <= 1) where initial selection is considered (methods recalcListItems and reset).

[Antonio Gomes]

Great explaination! pls add that to the ChangeLog, if you have not yet

(In reply to comment #8)
> (In reply to comment #7)
> > (From update of attachment 53368 [details] [details])
> > > This patch makes it possible for <select multiple> and <select size="n gt 1"> elements to have no selected option
> > > when rendered as menu lists.
> > 
> > I don't quite understand what is the problem and how this patch solves it.
> > Could you explain this in the ChangeLog too? You say "makes it possible" but
> > why, and how?
> 
> The target here is to have exactly the same behavior with and without
> NO_LISTBOX_RENDERING enabled. I mean, the very same <select> element in the
> very same html file must have the very same selection after loading the page
> with and without  NO_LISTBOX_RENDERING enabled.
> 
> The problem is that, for menu lists, if the selection is not indicated by the
> html file the first <option> will be selected after loading the page or
> reseting the form. Please consider bugzilla advanced search form:
> https://bugs.webkit.org/query.cgi?format=advanced . When the page is loaded
> many of its listboxes do not have a selected element (for example product,
> component, version). But when NO_LISTBOX_RENDERING is enabled the listboxes
> will became menu lists and then they will have an item selected after the page
> is loaded. That is the behavior difference that this patch corrects.
> 
> As when NO_LISTBOX_RENDERING is enabled usesMenuList() always returns true
> usesMenuList() cannot be used to decide about initial selection of the
> elements. This patch replaces (usesMenuLists()) by (!multiple && size <= 1)
> where initial selection is considered (methods recalcListItems and reset).

[Luiz Agostini]

Created attachment 53433 [details]
patch 3

[Antti Koivisto]

Comment on attachment 53433 [details]
patch 3

The code here is really convoluted, I wonder if there is a way to clean it up.

Anyway, this seems correct and shouldn't change behavior without NO_LISTBOX_RENDERING as far as I can see, so r=me

[WebKit Commit Bot]

Comment on attachment 53433 [details]
patch 3

Clearing flags on attachment: 53433

Committed r57714: <http://trac.webkit.org/changeset/57714>

[WebKit Commit Bot]

Comment on attachment 53433 [details]
patch 3

Rejecting patch 53433 from commit-queue.

Failed to run "['/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', '--force']" exit_code: 1
Last 500 characters of output:
++++++++
 3 files changed, 62 insertions(+), 4 deletions(-)
 create mode 100644 WebCore/manual-tests/no-listbox-rendering.html


-------------------------------------------------------------------
patching file WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file WebCore/dom/SelectElement.cpp
Hunk #1 FAILED at 270.
Hunk #2 FAILED at 499.
2 out of 2 hunks FAILED -- saving rejects to file WebCore/dom/SelectElement.cpp.rej
patching file WebCore/manual-tests/no-listbox-rendering.html

Full output: http://webkit-commit-queue.appspot.com/results/1533517

[Luiz Agostini]

landed in r57714.

[Simon Hausmann]

Revision 57714 cherry-picked into qtwebkit-2.0 with commit 41626128813739703756079cb7fa3e290274678c