Adoption and DocumentFragment, part two #819

annevk · 2020-01-10T15:10:11Z

This reverts commit c825cea and adds a new solution for DocumentFragment adoption on top. See #813 and #814 for context.

HTML: whatwg/html#5413

Tests: web-platform-tests/wpt#22504

jsdom: jsdom/jsdom#2925

Preview | Diff

See whatwg/dom#819. Verified with jsdom/jsdom#2925.

Complements whatwg/dom#819. Tests: web-platform-tests/wpt#22504.

annevk · 2020-03-30T13:15:27Z

Bugs (still the same):

domenic · 2020-04-27T22:53:21Z

Anything I can help with on the jsdom side, or on reviews, to land this and the associated HTML/WPT PRs?

annevk · 2020-11-06T12:39:55Z

Verifying that this is correct would help as well as a review. Perhaps @mfreed7 is interested in prototyping in Chromium?

mfreed7 · 2020-11-06T16:56:21Z

Verifying that this is correct would help as well as a review. Perhaps @mfreed7 is interested in prototyping in Chromium?

The change looks reasonable, but from #814 it might be tricky to get exactly right. I'm happy to prototype in Chromium, but I'd prefer to do that after jsdom has had a chance to confirm the change passes existing WPTs... @TimothyGu?

The DOM spec is currently broken because of changes around the adoption of nodes. There is an open PR which reverts some of these changes. This implements the same changes as in that PR to fix issues where some mutations would otherwise generate invalid mutation records.

dom.bs

meshl555

Noneo8

This reverts commit c825cea. See #813 and #814 for context.

Tests: web-platform-tests/wpt#22504. Corresponding HTML PR: whatwg/html#5413. Closes #813 and closes #814.

Complements whatwg/dom#819. Tests: web-platform-tests/wpt#22504.

rniwa · 2022-10-19T16:22:26Z

FWIW, I've implemented this new behavior in https://commits.webkit.org/252098@main

bigopon · 2022-10-25T21:51:57Z

dom.bs

+ <li>
+  <p>If <var>node</var> is a {{DocumentFragment}} <a for=/>node</a> and its
+  <a for=DocumentFragment>host</a> is non-null, then return <var>node</var>.
+
+  <p class=note>Unfortunately this does not throw for web compatibility.


What does "does not throw for web compatibility" mean? Any one who employs template.content node adoption before will now face with breakage with the sudden change of "whose host is non-null, then return".

If we no longer guarantee old API evolves without breaking, how do I find some rule of thumbs what is dangerous and what is not? An example issue is here aurelia/framework#1003

@bigopon before this change adoptNode() would have returned undefined which would result in undefined behavior as its IDL claims its return value is a Node.

From reading the issue you linked it seems that implementations never matched the specification and some applications (such as yours) relied on that. Perhaps we need to continue to allow that, but it would be a strange special case for HTMLTemplateElement's DocumentFragment usage as we cannot do the same thing if you were to pass a ShadowRoot for instance. It would also result in a rather weird HTMLTemplateElement instance, but since nothing much builds upon it maybe that is okay.

From reading the issue you linked it seems that implementations never matched the specification and some applications (such as yours) relied on that.

That's not true. The implementations were according to the spec until #754, that inserted a rule about template's fragment for the first time.
That rule returned undefined, there were some issues with that and the browsers didn't implemented (according to this ).
Then with #819 the spec changed again.
On July webkit implemented the new rule. Before one month Safari went public with the new version of webkit and many applications broke on production.
And since then we are trying to figure out what's going on.

So it sounds from the linked issues and comment like this change isn't web compatible as-is, right? Sounds like another approach is needed.

Any updates on this?
Right now our application is not working on Safari on production.
Webkit says that this is intentional behavior change and we don't know how to proceed to fix our application...

jded76 · 2022-11-05T12:21:57Z

Sorry for coming back on this again and again, but we have a problem in production with Safari and it seems that we got stuck...
Just ignoring it doesn't solve anything. I think a decision has to be made here soon...

bigopon · 2023-01-30T07:45:06Z

Is there a process that I need to follow or do I just create a PR to fix this? The easiest option is to revert the change and find a more reasonable one later.

bigopon · 2023-02-15T08:08:27Z

It's been awhile without answer, where can I shoot my concerns over this issues? It's breaking both apps and hearts.

mfreed7 · 2023-02-22T18:58:54Z

It's been awhile without answer, where can I shoot my concerns over this issues? It's breaking both apps and hearts.

I believe you're referring to the WebKit/Safari behavior described here, correct? https://bugs.webkit.org/show_bug.cgi?id=246899

If so, that bug is the better place to raise the issue, since this issue is about the spec. That bug is informative about the direction the spec needs to take, though, since it seems the proposed direction isn't web compatible.

karlcow · 2023-03-10T18:50:27Z

so adoptNode behavior has been reverted in WebKit
WebKit/WebKit#11347
because of aurelia/framework#1003
WebKit being the only to have adopted the new behavior, it was having the burden of the breakage.

There is a need for a better path forward.

bigopon · 2023-03-10T20:52:30Z

There is a need for a better path forward.

What we want is the ability to adopt the document fragment. Currently adopting the template element doesn't adopt the document fragment itself. What if we do that?

I dont know all the reasons why the fragment of the template element isn't associated with the document that created it, but if calling adoptNode explicitly on template does it then maybe this behavior is safe to go in?

pshaughn mentioned this pull request Jan 24, 2020

Run adopt as part of insert servo/servo#25265

Open

pshaughn mentioned this pull request Feb 15, 2020

Run adopt as part of insert servo/servo#25460

Closed

4 tasks

annevk mentioned this pull request Mar 27, 2020

DOM: change how adoption works for DocumentFragment jsdom/jsdom#2925

Closed

annevk force-pushed the annevk/revert-abort-changes branch from 32cfc18 to 6eb77d0 Compare March 27, 2020 19:01

annevk added a commit to web-platform-tests/wpt that referenced this pull request Mar 27, 2020

DOM: latest DocumentFragment adoption changes

ebc83f7

See whatwg/dom#819. Verified with jsdom/jsdom#2925.

This was referenced Mar 27, 2020

DOM: latest DocumentFragment adoption changes web-platform-tests/wpt#22504

Open

replaceChild() has odd mutation observer behavior after #754 #814

Open

annevk added a commit to whatwg/html that referenced this pull request Mar 28, 2020

Align with DOM DocumentFragment adoption changes

2dadc91

Complements whatwg/dom#819. Tests: web-platform-tests/wpt#22504.

annevk mentioned this pull request Mar 28, 2020

Align with DOM DocumentFragment adoption changes whatwg/html#5413

Open

3 tasks

annevk changed the title ~~Revert "Run adopt as part of insert"~~ Adoption and DocumentFragment, part two Mar 30, 2020

annevk force-pushed the annevk/revert-abort-changes branch from 8dafaf7 to 9a1e814 Compare November 6, 2020 12:39

annevk mentioned this pull request Nov 6, 2020

Remove Goals and Historical sections #918

Merged

annevk mentioned this pull request Dec 1, 2020

What should adoptNode return for a DocumentFragment with a non-null host? #930

Closed

annevk force-pushed the annevk/revert-abort-changes branch from 9a1e814 to 5c12594 Compare December 1, 2020 09:40

Base automatically changed from master to main January 15, 2021 07:32

annevk force-pushed the annevk/revert-abort-changes branch from 5c12594 to 77ad4ca Compare January 19, 2021 11:18

bwrrp mentioned this pull request Jan 31, 2021

Update to latest specs, add StaticRange and replaceChildren bwrrp/slimdom.js#129

Closed

annevk mentioned this pull request Aug 10, 2021

adoptNode() should always return a node #1006

Closed

rniwa reviewed Jul 2, 2022

View reviewed changes

dom.bs Outdated Show resolved Hide resolved

meshl555 reviewed Jul 16, 2022

View reviewed changes

This comment was marked as spam.

Sign in to view

annevk added 2 commits October 18, 2022 11:13

Revert "Run adopt as part of insert"

1b137ba

This reverts commit c825cea. See #813 and #814 for context.

Make adoption account for DocumentFragment nodes with hosts

7afc950

Tests: web-platform-tests/wpt#22504. Corresponding HTML PR: whatwg/html#5413. Closes #813 and closes #814.

annevk force-pushed the annevk/revert-abort-changes branch from 77ad4ca to 7afc950 Compare October 18, 2022 09:18

annevk requested review from rniwa and mfreed7 October 18, 2022 09:19

annevk added a commit to whatwg/html that referenced this pull request Oct 18, 2022

Align with DOM DocumentFragment adoption changes

b4ec415

Complements whatwg/dom#819. Tests: web-platform-tests/wpt#22504.

3cp mentioned this pull request Oct 23, 2022

Many aurelia libraries are broken in iOS 16 aurelia/framework#1003

Closed

bigopon reviewed Oct 25, 2022

View reviewed changes

bigopon mentioned this pull request Nov 3, 2022

In aurelia template, it shows "Cannot read properties of null (reading 'insertBefore')" error. aurelia/templating#709

Closed

bigopon mentioned this pull request Nov 28, 2022

fix: vNext fast-router templates not attaching on safari 16+ microsoft/fast#6537

Closed

jattasNI mentioned this pull request Jan 24, 2023

nimble-table data doesn't render in Safari ni/nimble#984

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adoption and DocumentFragment, part two #819

Adoption and DocumentFragment, part two #819

annevk commented Jan 10, 2020 •

edited by pr-preview bot

Loading

annevk commented Mar 30, 2020

domenic commented Apr 27, 2020

annevk commented Nov 6, 2020 •

edited

Loading

mfreed7 commented Nov 6, 2020

meshl555 left a comment

This comment was marked as spam.

rniwa commented Oct 19, 2022

bigopon Oct 25, 2022 •

edited

Loading

annevk Oct 26, 2022

jded76 Oct 27, 2022 •

edited

Loading

mfreed7 Oct 27, 2022

jded76 Oct 31, 2022

jded76 commented Nov 5, 2022

bigopon commented Jan 30, 2023

bigopon commented Feb 15, 2023

mfreed7 commented Feb 22, 2023

karlcow commented Mar 10, 2023

bigopon commented Mar 10, 2023

Adoption and DocumentFragment, part two #819

Are you sure you want to change the base?

Adoption and DocumentFragment, part two #819

Conversation

annevk commented Jan 10, 2020 • edited by pr-preview bot Loading

annevk commented Mar 30, 2020

domenic commented Apr 27, 2020

annevk commented Nov 6, 2020 • edited Loading

mfreed7 commented Nov 6, 2020

meshl555 left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

rniwa commented Oct 19, 2022

bigopon Oct 25, 2022 • edited Loading

Choose a reason for hiding this comment

annevk Oct 26, 2022

Choose a reason for hiding this comment

jded76 Oct 27, 2022 • edited Loading

Choose a reason for hiding this comment

mfreed7 Oct 27, 2022

Choose a reason for hiding this comment

jded76 Oct 31, 2022

Choose a reason for hiding this comment

jded76 commented Nov 5, 2022

bigopon commented Jan 30, 2023

bigopon commented Feb 15, 2023

mfreed7 commented Feb 22, 2023

karlcow commented Mar 10, 2023

bigopon commented Mar 10, 2023

annevk commented Jan 10, 2020 •

edited by pr-preview bot

Loading

annevk commented Nov 6, 2020 •

edited

Loading

bigopon Oct 25, 2022 •

edited

Loading

jded76 Oct 27, 2022 •

edited

Loading