Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Safari 16 crashes on in-template DOM manipulation #5147

Closed
jded76 opened this issue Oct 22, 2022 · 4 comments · Fixed by #5158 or jquery/sizzle#490
Closed

Safari 16 crashes on in-template DOM manipulation #5147

jded76 opened this issue Oct 22, 2022 · 4 comments · Fixed by #5158 or jquery/sizzle#490
Assignees
Labels
Milestone

Comments

@jded76
Copy link

jded76 commented Oct 22, 2022

Description

After the new IOS 16, Safari changed the behavior of document.adoptNode when called on a template. It's not changing the ownerDocument any more. It leaves this on an empty document (about:blank).

This causing an error at the function contains at this line, because the documentElement property is null on an empty document (about:blank).

If you change this line from
var adown = a.nodeType === 9 ? a.documentElement : a, bup = b && b.parentNode;
to
var adown = a.nodeType === 9 && a.documentElement ? a.documentElement : a, bup = b && b.parentNode;
it's working again.

More details can be found here and aurelia/framework#1003.

Link to test case

You can see the problem in this fiddler if you try it with Safari 16+.
If you press "add" and then "remove" you get the error.

@jded76
Copy link
Author

jded76 commented Oct 23, 2022

I made a repository if you like to test it without fiddler.

If you change this line to use the local jquery file <script src="jquery-3.5.1.js"></script> the error is gone.

@mgol mgol added the Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. label Oct 24, 2022
@timmywil timmywil removed the Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. label Oct 24, 2022
@timmywil timmywil added this to the 3.6.2 milestone Oct 24, 2022
@mgol mgol self-assigned this Nov 14, 2022
@mgol mgol added the Selector label Nov 14, 2022
@mgol mgol changed the title Problem with Safari 16+ Safari 16 crashes on in-template DOM manipulation Nov 14, 2022
mgol added a commit to mgol/jquery that referenced this issue Nov 14, 2022
The `<template/>` element `contents` property is a document fragment that may
have a `null` `documentElement`. In Safari 16 this happens in more cases due
to recent spec changes - in particular, even if that document fragment is
explicitly adopted into an outer document. We're testing both of those cases
now.

The crash used to happen in `jQuery.contains`. As it turns out, we don't need
to query the supposed container `documentElement` if it has the
`Node.DOCUMENT_NODE` (9) `nodeType`; we can call `.contains()` directly on
the `document`. That avoids the crash.

Fixes jquerygh-5147
mgol added a commit that referenced this issue Nov 14, 2022
The `<template/>` element `contents` property is a document fragment that may
have a `null` `documentElement`. In Safari 16 this happens in more cases due
to recent spec changes - in particular, even if that document fragment is
explicitly adopted into an outer document. We're testing both of those cases
now.

The crash used to happen in `jQuery.contains`. As it turns out, we don't need
to query the supposed container `documentElement` if it has the
`Node.DOCUMENT_NODE` (9) `nodeType`; we can call `.contains()` directly on
the `document`. That avoids the crash.

Fixes gh-5147
Closes gh-5158
@mgol
Copy link
Member

mgol commented Nov 14, 2022

Reopening until the fix lands in Sizzle and - then - on 3.x-stable.

@mgol mgol reopened this Nov 14, 2022
@mgol mgol closed this as completed in 3299236 Nov 14, 2022
@mgol mgol reopened this Nov 14, 2022
mgol added a commit to mgol/sizzle that referenced this issue Nov 15, 2022
The `<template/>` element `contents` property is a document fragment that may
have a `null` `documentElement`. In Safari 16 this happens in more cases due
to recent spec changes - in particular, even if that document fragment is
explicitly adopted into an outer document. We're testing both of those cases
now.

The above behavior made `Sizzle.contains` crash when run on certain elements
within the `<template/>` element. As it turns out, we don't need
to query the supposed container `documentElement` if it has the
`Node.DOCUMENT_NODE` (9) `nodeType`; we can call `.contains()` directly on
the `document`. That avoids the crash.

However, we still need to fall back to `documentElement` if one is
defined as IE <9 have a broken `.contains()` on the document.

Fixes jquery/jquery#5147
Ref jquery/jquery#5158
mgol added a commit to mgol/jquery that referenced this issue Nov 15, 2022
The `<template/>` element `contents` property is a document fragment that may
have a `null` `documentElement`. In Safari 16 this happens in more cases due
to recent spec changes - in particular, even if that document fragment is
explicitly adopted into an outer document. We're testing both of those cases
now.

The crash used to happen in `jQuery.contains` which is an alias for
`Sizzle.contains` in jQuery 3.x.

The Sizzle fix is at jquery/sizzle#490. This PR pulls the new Sizzle
version into jQuery's `3.x-stable` branch and backports one test from
jquerygh-5158.

Fixes jquerygh-5147
Ref jquery/sizzle/pull/490
Ref jquerygh-5158
@mgol
Copy link
Member

mgol commented Nov 15, 2022

Landed on main in 3299236.

The 3.x-stable part of the fix needs to first happen in Sizzle.

Sizzle PR: jquery/sizzle#490
3.x-stable PR (will fail until the Sizzle upgrade is incorporated into it): #5159

@mgol mgol reopened this Nov 16, 2022
@mgol mgol reopened this Nov 16, 2022
mgol added a commit to mgol/jquery that referenced this issue Nov 16, 2022
mgol added a commit to mgol/jquery that referenced this issue Nov 16, 2022
The `<template/>` element `contents` property is a document fragment that may
have a `null` `documentElement`. In Safari 16 this happens in more cases due
to recent spec changes - in particular, even if that document fragment is
explicitly adopted into an outer document. We're testing both of those cases
now.

The crash used to happen in `jQuery.contains` which is an alias for
`Sizzle.contains` in jQuery 3.x.

The Sizzle fix is at jquery/sizzle#490. This PR pulls the new Sizzle
version into jQuery's `3.x-stable` branch and backports one test from
jquerygh-5158.

Fixes jquerygh-5147
Closes jquerygh-5159
Ref jquery/sizzle#490
Ref jquerygh-5158
mgol added a commit to mgol/jquery that referenced this issue Nov 16, 2022
The `<template/>` element `contents` property is a document fragment that may
have a `null` `documentElement`. In Safari 16 this happens in more cases due
to recent spec changes - in particular, even if that document fragment is
explicitly adopted into an outer document. We're testing both of those cases
now.

The crash used to happen in `jQuery.contains` which is an alias for
`Sizzle.contains` in jQuery 3.x.

The Sizzle fix is at jquery/sizzle#490, released in Sizzle `2.3.8`. This
version of Sizzle is included in the parent commit.

Fixes jquerygh-5147
Closes jquerygh-5159
Ref jquery/sizzle#490
Ref jquerygh-5158
mgol added a commit to mgol/jquery that referenced this issue Nov 16, 2022
The `<template/>` element `contents` property is a document fragment that may
have a `null` `documentElement`. In Safari 16 this happens in more cases due
to recent spec changes - in particular, even if that document fragment is
explicitly adopted into an outer document. We're testing both of those cases
now.

The crash used to happen in `jQuery.contains` which is an alias for
`Sizzle.contains` in jQuery 3.x.

The Sizzle fix is at jquery/sizzle#490, released in Sizzle `2.3.8`. This
version of Sizzle is included in the parent commit.

A fix similar to the one from jquerygh-5158 has also been applied here to the
`selector-native` version.

Fixes jquerygh-5147
Closes jquerygh-5159
Ref jquery/sizzle#490
Ref jquerygh-5158
mgol added a commit that referenced this issue Nov 16, 2022
The `<template/>` element `contents` property is a document fragment that may
have a `null` `documentElement`. In Safari 16 this happens in more cases due
to recent spec changes - in particular, even if that document fragment is
explicitly adopted into an outer document. We're testing both of those cases
now.

The crash used to happen in `jQuery.contains` which is an alias for
`Sizzle.contains` in jQuery 3.x.

The Sizzle fix is at jquery/sizzle#490, released in Sizzle `2.3.8`. This
version of Sizzle is included in the parent commit.

A fix similar to the one from gh-5158 has also been applied here to the
`selector-native` version.

Fixes gh-5147
Closes gh-5159
Ref jquery/sizzle#490
Ref gh-5158
@mgol
Copy link
Member

mgol commented Nov 16, 2022

Landed on main in 3299236 and on 3.x-stable in a1b7ae3 (commit updating Sizzle, includes the fix from jquery/sizzle@759acc7) & 5318e31. Closing.

@mgol mgol closed this as completed Nov 16, 2022
mgol added a commit to mgol/jquery that referenced this issue Nov 17, 2022
The `<template/>` element `contents` property is a document fragment that may
have a `null` `documentElement`. In Safari 16 this happens in more cases due
to recent spec changes - in particular, even if that document fragment is
explicitly adopted into an outer document. We're testing both of those cases
now.

The crash used to happen in `jQuery.contains`. As it turns out, we don't need
to query the supposed container `documentElement` if it has the
`Node.DOCUMENT_NODE` (9) `nodeType`; we can call `.contains()` directly on
the `document`. That avoids the crash.

Fixes jquerygh-5147
Closes jquerygh-5158

(cherry picked from commit 3299236)
mgol added a commit to mgol/jquery that referenced this issue Dec 1, 2022
The `<template/>` element `contents` property is a document fragment that may
have a `null` `documentElement`. In Safari 16 this happens in more cases due
to recent spec changes - in particular, even if that document fragment is
explicitly adopted into an outer document. We're testing both of those cases
now.

The crash used to happen in `jQuery.contains`. As it turns out, we don't need
to query the supposed container `documentElement` if it has the
`Node.DOCUMENT_NODE` (9) `nodeType`; we can call `.contains()` directly on
the `document`. That avoids the crash.

Fixes jquerygh-5147
Closes jquerygh-5158

(cherry picked from commit 3299236)
mgol added a commit to mgol/jquery that referenced this issue Dec 13, 2022
The `<template/>` element `contents` property is a document fragment that may
have a `null` `documentElement`. In Safari 16 this happens in more cases due
to recent spec changes - in particular, even if that document fragment is
explicitly adopted into an outer document. We're testing both of those cases
now.

The crash used to happen in `jQuery.contains`. As it turns out, we don't need
to query the supposed container `documentElement` if it has the
`Node.DOCUMENT_NODE` (9) `nodeType`; we can call `.contains()` directly on
the `document`. That avoids the crash.

Fixes jquerygh-5147
Closes jquerygh-5158

(cherry picked from commit 3299236)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment