Closed Bug 1214164 Opened 6 years ago Closed 6 years ago
Only <options> that are children of <select> or children of <optgroup> children of <select> should be honored
Spec: """ The list of options for a select element consists of all the option element children of the select element, and all the option element children of all the optgroup element children of the select element, in tree order. """ https://html.spec.whatwg.org/#concept-select-option-list Test-case: data:text/html,<!doctype html> <script> var select = document.createElement("select"); select.appendChild(document.createElement("optgroup")); select.firstChild.appendChild(document.createElement("optgroup")); select.firstChild.firstChild.appendChild(document.createElement("option")); document.documentElement.textContent = select.options.length; </script> IE11 outputs 0, per spec. Gecko and Chrome output 1. But tkent says Blink now follows the spec <https://github.com/whatwg/html/issues/249#issuecomment-147635203>. The DOMs in question cannot be produced by the parser, so behavior is essentially inconsequential, so I think we should follow the path of least resistance and change to match the spec. annevk is inclined to agree <https://github.com/whatwg/html/issues/249#issuecomment-147638810>.
So there are no nested optgroups? At one point I thought there were... but discussion in bug 79431 suggests they were just a proposal that never actually materialized, If there aren't (and the special handling of option/optgroup in nsCSSFrameConstructor::AddFrameConstructionItemsInternal suggests there aren't in display terms), then we probably need to fix (simplify, likely) at least the following: HTMLSelectElement::InsertOptionsIntoListRecurse HTMLSelectElement::RemoveOptionsFromListRecurse AddOptionsRecurse in HTMLSelectElement.cpp VerifyOptionsRecurse in same HTMLOptGroupElement::GetSelect
Duplicate of this bug: 79431
Lightly tested, and I don't understand the code, so beware. It's likely that further simplifications are possible somewhere here. https://treeherder.mozilla.org/#/jobs?repo=try&revision=af2e1e7c53ab
Attachment #8673103 - Flags: review?(bzbarsky)
Comment on attachment 8673103 [details] [diff] [review] Patch > HTMLSelectElement::RemoveOptionsFromList(nsIContent* aOptions, >+ nsCOMPtr<nsIDOMHTMLOptionElement> optElement(do_QueryInterface(aOptions)); I know that's what it used to do, but this can just be HTMLOptionElement::FromContent, right? >+ optElement = do_QueryInterface(child); And here. >+HTMLSelectElement::VerifyOptionsArray() >+ nsCOMPtr<nsIDOMHTMLOptionElement> opt = do_QueryInterface(child); And here. >+ nsCOMPtr<nsIDOMHTMLOptionElement> opt = do_QueryInterface(grandchild); And here. r=me with those nits fixed.
Attachment #8673103 - Flags: review?(bzbarsky) → review+
That still has the "nsCOMPtr<nsIDOMHTMLOptionElement> opt = do_QueryInterface(grandchild);" bit...
Attachment #8673226 - Attachment is obsolete: true
Posted the site compatibility document just in case. https://www.fxsitecompat.com/en-US/docs/2015/nested-optgroups-are-no-longer-allowed/
You need to log in before you can comment on or make changes to this bug.