| Summary: | REGRESSION(r102741): [Forms] In selects, when disabled, browser skips first option if not in optgroup, then selects first option in optgroup | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | yosin | ||||||
| Component: | Forms | Assignee: | yosin | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | cmarcelo, dino, eric, macpherson, menard, mifenton, tkent, webkit.review.bot | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| URL: | http://jsfiddle.net/UZBKd/ | ||||||||
| Bug Depends on: | 93538 | ||||||||
| Bug Blocks: | |||||||||
| Attachments: |
|
||||||||
This bug is imported from http://crbug.com/139823 The sample URI, http://jsfiddle.net/UZBKd/, should display "1" as other browsers, FireFox, IE, and Opera. Created attachment 155978 [details]
Patch 1
Comment on attachment 155978 [details]
Patch 1
Could you review this patch?
Thanks in advance.
Comment on attachment 155978 [details] Patch 1 View in context: https://bugs.webkit.org/attachment.cgi?id=155978&action=review Need ChangeLog improvement > Source/WebCore/ChangeLog:9 > + This patch changes implementation of HTMLOptionElement::disabled() to > + follow HTML5 specification, the option element is disabled if option This is confusing. It's unclear that what part of the HTML5 specification it is. You should write ":disabled pseudo class" or add a link to the concept "disabled" [1] because "disabled" IDL attribute is different from it. [1] http://www.whatwg.org/specs/web-apps/current-work/multipage/the-button-element.html#concept-option-disabled > Source/WebCore/ChangeLog:15 > + Before this patch, HTMLSelectElement::recalcListItems() didn't pick What do you mean by 'pick'? Created attachment 155986 [details]
Patch 2
Comment on attachment 155986 [details]
Patch 2
Could you review this patch?
Thanks in advance.
= Changes since the last review =
* Update Source/WebCore/ChangeLog as suggested.
Comment on attachment 155986 [details]
Patch 2
looks good.
Comment on attachment 155986 [details] Patch 2 Clearing flags on attachment: 155986 Committed r124416: <http://trac.webkit.org/changeset/124416> All reviewed patches have been landed. Closing bug. This causes fast/forms/basic-selects.html to fail on Mac ports. I see the commit log says "Note: We need to rebaseline for all ports expect for Chromium-Linux.". Did this ever happen? (In reply to comment #10) > This causes fast/forms/basic-selects.html to fail on Mac ports. > > I see the commit log says "Note: We need to rebaseline for all ports expect for Chromium-Linux.". Did this ever happen? No, I updated for Chromium ports, but not for others. Could you update expected texts and images for Mac port? Thanks in advance. |
The select element should pick up not "disable" option element for selected option when there is no "selected" option. HTMLSelectElement::recalcListItems() does this by checking HTMLOptionElement::option(). It seems implementation of HTMLOPtionElement::option() isn't matched to HTML5 specification[1]: An option element is disabled if its disabled attribute is present or if it is a child of an optgroup element whose disabled attribute is present. However, current implementation checks "disabled" attribute regardless element tag name: bool HTMLOptionElement::disabled() const { return ownElementDisabled() || (parentNode() && parentNode()->isHTMLElement() && static_cast<HTMLElement*>(parentNode())->disabled()); } In sample URI[2], the parent element is the "select" element having "disabled" attribute. So, HTMLOptionElement::disable() return true rather than false. This is the reason we pick up the first "option" element in the first "optgroup" element. = References = [1] http://www.whatwg.org/specs/web-apps/current-work/multipage/the-button-element.html#the-option-element [2] http://jsfiddle.net/UZBKd/