Closed Bug 1365399 Opened 5 years ago Closed 5 years ago

Option element constructor is not per spec

Categories

(Core :: DOM: Core & HTML, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: d, Assigned: jessica)

References

Details

Attachments

(1 file)

In particular it appears to handle defaultSelected incorrectly and sets dirtiness incorrectly. See test at http://w3c-test.org/html/semantics/forms/the-option-element/option-element-constructor.html and spec at https://html.spec.whatwg.org/#dom-option

This is probably related to (a dupe of?) https://bugzilla.mozilla.org/show_bug.cgi?id=1300282
Priority: -- → P2
Assignee: nobody → jjong
Attached patch patch, v1.Splinter Review
Some of the work has been done in Bug 1365598.
This bug fixes this statement in the spec (https://html.spec.whatwg.org/#dom-option):
  If selected is true, then set option's selectedness to true; otherwise set its
  selectedness to false (even if defaultSelected is true).
And also reset the dirtiness.

This patch also reverts the changes in Bug 927796 and 942648, which was added
because IsSelected() returned inaccurately (called from SelectSomething()).
Verified that the tests added in Bug 927796 and 942648 do pass.
Attachment #8871643 - Flags: review?(bugs)
Comment on attachment 8871643 [details] [diff] [review]
patch, v1.

So mSelectedChanged is used only by HTMLOptionElement::SetSelectedInternal now and checked in HTMLOptionElement::BeforeSetAttr
Attachment #8871643 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #2)
> Comment on attachment 8871643 [details] [diff] [review]
> patch, v1.
> 
> So mSelectedChanged is used only by HTMLOptionElement::SetSelectedInternal
> now and checked in HTMLOptionElement::BeforeSetAttr

Exactly, and cleared (set to false) in Option constructor (added in this patch) and in HTMLSelectElement::Reset.
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/933ff638ea34
Handle selectedness and dirtiness correctly in Option constructor. r=smaug
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/933ff638ea34
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.