Bug 250695
| Summary: | Form association by HTML parser should not work if the form element is not in the document tree | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Ahmad Saleem <ahmad.saleem792> |
| Component: | Forms | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | akeerthi, ashvayka, cdumez, rniwa, webkit-bug-importer, wenson_hsieh |
| Priority: | P2 | Keywords: | InRadar |
| Version: | Safari Technology Preview | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
Ahmad Saleem
Hi Team,
While going through Blink's commit, I came across another failing test:
NOTE - I took updated copy of test from source.chromium.org rather than from commit.
Test Case - https://jsfiddle.net/tq84a3pv/show
^ Chrome Canary 111 and Firefox Nightly 110 passes all tests while Safari Technology 161 fails following:
Testing LABEL
FAIL formOwner should be not defined. Was defined.
Testing INPUT
FAIL formOwner should be not defined. Was defined.
Testing LABEL
FAIL formOwner should be not defined. Was defined.
Testing INPUT
FAIL formOwner should be not defined. Was defined.
Testing DIV
FAIL formOwner should be not defined. Was defined.
Testing A
FAIL formOwner should be not defined. Was defined.
Testing P
FAIL formOwner should be not defined. Was defined.
______
Blink Commit - https://chromium.googlesource.com/chromium/blink/+/ca6002a6e198c84792fed0eb5251a027d7611e9d
Just wanted to raise so we can fix it. I haven't checked whether 1-1 merge is possible or not.
Thanks!
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Alexey Shvayka
That's a great find, thank you Ahmad! It's especially important wrt shipping form-associated custom elements.
Blinks diff is useful yet 1-1 merge doesn't seem possible. Adding this to my backlog of form-associated follow-ups.
Radar WebKit Bug Importer
<rdar://problem/104582632>
Ahmad Saleem
(In reply to Alexey Shvayka from comment #1)
> That's a great find, thank you Ahmad! It's especially important wrt shipping
> form-associated custom elements.
>
> Blinks diff is useful yet 1-1 merge doesn't seem possible. Adding this to my
> backlog of form-associated follow-ups.
Alexey, I checked on WebKit Turnk and it seems to pass all tests and also STP162, do we need anything else or we can close this or just import test case into our tree?
Thanks!
Alexey Shvayka
(In reply to Ahmad Saleem from comment #3)
> (In reply to Alexey Shvayka from comment #1)
> > That's a great find, thank you Ahmad! It's especially important wrt shipping
> > form-associated custom elements.
> >
> > Blinks diff is useful yet 1-1 merge doesn't seem possible. Adding this to my
> > backlog of form-associated follow-ups.
>
> Alexey, I checked on WebKit Turnk and it seems to pass all tests and also
> STP162, do we need anything else or we can close this or just import test
> case into our tree?
>
> Thanks!
Thank you Ahmad for following up on this! I've also checked the code and it looks like all isConnected() checks are in place on Trunk.
This was probably accidentally fixed along with form-associated custom elements implementation.
It would be extremely valuable to import this test so we won't accidentally regress when landing follow-ups.
Ahmad Saleem
(In reply to Alexey Shvayka from comment #4)
> (In reply to Ahmad Saleem from comment #3)
> > (In reply to Alexey Shvayka from comment #1)
> > > That's a great find, thank you Ahmad! It's especially important wrt shipping
> > > form-associated custom elements.
> > >
> > > Blinks diff is useful yet 1-1 merge doesn't seem possible. Adding this to my
> > > backlog of form-associated follow-ups.
> >
> > Alexey, I checked on WebKit Turnk and it seems to pass all tests and also
> > STP162, do we need anything else or we can close this or just import test
> > case into our tree?
> >
> > Thanks!
>
> Thank you Ahmad for following up on this! I've also checked the code and it
> looks like all isConnected() checks are in place on Trunk.
>
> This was probably accidentally fixed along with form-associated custom
> elements implementation.
>
> It would be extremely valuable to import this test so we won't accidentally
> regress when landing follow-ups.
Will do PR to import test. Don't worry, I got it. Thanks!
Ahmad Saleem
PR - https://github.com/WebKit/WebKit/pull/9823
EWS
Committed 260053@main (52f00031f862): <https://commits.webkit.org/260053@main>
Reviewed commits have been landed. Closing PR #9823 and removing active labels.