RESOLVED FIXED 190227
input.checked is incorrect while we're parsing its children
https://bugs.webkit.org/show_bug.cgi?id=190227
Summary input.checked is incorrect while we're parsing its children
Chris Dumez
Reported 2018-10-02 16:44:57 PDT
input.checked is incorrect while we're parsing its children.
Attachments
Patch (5.01 KB, patch)
2018-10-02 16:52 PDT, Chris Dumez
no flags
Patch (5.13 KB, patch)
2018-10-02 16:58 PDT, Chris Dumez
no flags
Chris Dumez
Comment 1 2018-10-02 16:52:50 PDT
Chris Dumez
Comment 2 2018-10-02 16:58:58 PDT
Darin Adler
Comment 3 2018-10-02 18:25:48 PDT
Comment on attachment 351458 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=351458&action=review > Source/WebCore/ChangeLog:13 > + In this patch, we update the checked state as soon as the 'checked' attribute is set, when we know > + that no form state to restore. What about in the case where there is form state to restore? Do we have a test covering that? Is our behavior already what we would like it to be in that case?
Chris Dumez
Comment 4 2018-10-03 07:40:12 PDT
(In reply to Darin Adler from comment #3) > Comment on attachment 351458 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=351458&action=review > > > Source/WebCore/ChangeLog:13 > > + In this patch, we update the checked state as soon as the 'checked' attribute is set, when we know > > + that no form state to restore. > > What about in the case where there is form state to restore? Do we have a > test covering that? Is our behavior already what we would like it to be in > that case? Yes, as I stated in my change log, we have already a test case covering that and it is still passing.
Chris Dumez
Comment 5 2018-10-03 08:39:41 PDT
(In reply to Chris Dumez from comment #4) > (In reply to Darin Adler from comment #3) > > Comment on attachment 351458 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=351458&action=review > > > > > Source/WebCore/ChangeLog:13 > > > + In this patch, we update the checked state as soon as the 'checked' attribute is set, when we know > > > + that no form state to restore. > > > > What about in the case where there is form state to restore? Do we have a > > test covering that? Is our behavior already what we would like it to be in > > that case? > > Yes, as I stated in my change log, we have already a test case covering that > and it is still passing. And to answer your second question: No, I do not think our behavior is what we would like it to be in the less common state restoration case because it is non-standard behavior. That said, making our behavior standard in the state restoration case while not breaking state restoration is going to be more work and I did not have time to look into this part further. Also note that AFAICT, Blink also does not seem to reflect the correct checked state in the state restoration case, which reduces the risk of Web breakage.
WebKit Commit Bot
Comment 6 2018-10-03 09:02:42 PDT
Comment on attachment 351458 [details] Patch Clearing flags on attachment: 351458 Committed r236795: <https://trac.webkit.org/changeset/236795>
WebKit Commit Bot
Comment 7 2018-10-03 09:02:44 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 8 2018-10-03 09:03:36 PDT
Chris Dumez
Comment 9 2018-10-16 16:30:15 PDT
Comment on attachment 351458 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=351458&action=review > Source/WebCore/html/HTMLInputElement.cpp:-749 > - m_reflectsCheckedAttribute = true; I mistakenly dropped this because it looked like a no-op given the if check above. However, the call to setChecked() sets the attribute to false so we do need to reset the flag to true here. Fixing via https://bugs.webkit.org/show_bug.cgi?id=190651.
Note You need to log in before you can comment on or make changes to this bug.