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

Description Ahmad Saleem 2023-01-16 18:05:34 PST

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!

Comment 1 Alexey Shvayka 2023-01-17 10:17:00 PST

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.

Comment 2 Radar WebKit Bug Importer 2023-01-23 18:06:17 PST

<rdar://problem/104582632>

Comment 3 Ahmad Saleem 2023-02-02 04:17:08 PST

(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!

Comment 4 Alexey Shvayka 2023-02-08 07:38:38 PST

(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.

Comment 5 Ahmad Saleem 2023-02-08 10:06:32 PST

(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!

Comment 6 Ahmad Saleem 2023-02-08 10:20:46 PST

PR - https://github.com/WebKit/WebKit/pull/9823

Comment 7 EWS 2023-02-09 00:14:27 PST

Committed 260053@main (52f00031f862): <https://commits.webkit.org/260053@main>

Reviewed commits have been landed. Closing PR #9823 and removing active labels.