WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(5.13 KB, patch)
2018-10-02 16:58 PDT
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Chris Dumez
Comment 1
2018-10-02 16:52:50 PDT
Created
attachment 351457
[details]
Patch
Chris Dumez
Comment 2
2018-10-02 16:58:58 PDT
Created
attachment 351458
[details]
Patch
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
<
rdar://problem/44975225
>
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.
Top of Page
Format For Printing
XML
Clone This Bug