Core: Make isAttached work with iOS 10.0-10.2 #4360
Conversation
|
We won't support iOS 10.0-10.2 in jQuery 4 so this doesn't need to land on |
|
fwiw, please note this is not only limited to iOS 10.0-10.2, the issue also happens under other platforms. Here are some desktop/mobile browsers we've detected:
I.e. basically anything using Chromium 53.x as well it seems ... |
|
I just ran into this same issue when upgrading jQuery in an Electron-based app. ( |
| // Support: iOS 10.0-10.2 only | ||
| // Early iOS 10 versions support `attachShadow` but not `getRootNode`, | ||
| // leading to errors. We need to check for `getRootNode`. | ||
| if ( documentElement.getRootNode ) { |
There was a problem hiding this comment.
According to MDN (and my previous post), this is not only limited to iOS:
There was a problem hiding this comment.
We don't officially support Chrome 53 (see https://jquery.com/browser-support/) so I didn't check that.
There was a problem hiding this comment.
I need to check what's up with Firefox, thanks for the pointer!
There was a problem hiding this comment.
Glad to be of help, and thanks for enlightening me about the officially supported versions, missed that sorry.
test/unit/css.js
Outdated
| } ); | ||
|
|
||
| QUnit[ document.body.attachShadow ? "test" : "skip" ]( "show/hide shadow child nodes", function( assert ) { | ||
| QUnit[ document.body.getRootNode ? "test" : "skip" ]( "show/hide shadow child nodes", function( assert ) { |
There was a problem hiding this comment.
As per the browser compatibility table from MDN i posted above, this test will fail under Firefox 53-63, these versions does support getRootNode but not attachShadow, so since this test rely on the later this change should not be needed.
There was a problem hiding this comment.
Just checking for attachShadow here would run the test in iOS 10.0 where it'd fail. We either need to check both or blacklist specific browsers by user agent (as it's just tests).
There was a problem hiding this comment.
Thanks for the pointer, though! I didn't notice these tests would fail in Firefox 60 ESR.
There was a problem hiding this comment.
Whoops, got it now after inspecting further... getNodeRoot is not used here, but will get invoked within the library code when swapping the display style 👍
So... yeah, this may turns tricky for Firefox.
mgol
force-pushed
the
ios10-isattached
branch
2 times, most recently
from
4f115eb to
db75f80
Compare
|
PR updated, thank you @diegocr for noticing test problems related to Firefox 60 ESR! I think the fact that in Firefox 60 we now use |
diegocr
left a comment
diegocr
left a comment
There was a problem hiding this comment.
This looks good to me, someone else more familiar with the codebase may want to double check though :-)
The test for Shadow DOM v1 support has been changed to rely on the presence of `documentElement.getRootNode` as iOS 10.0-10.2 supports `attachShadow` but doesn't support `getRootNode`. No new test is necessary - iOS 10.0 fails lots of our test suite because of this bug. Fixes jquerygh-4356
5 participants
Summary
The test for Shadow DOM v1 support has been changed to rely on the presence of
documentElement.getRootNodeas iOS 10.0-10.2 supportsattachShadowbutdoesn't support
getRootNode.No new test is necessary - iOS 10.0 fails lots of our test suite because of
this bug.
Fixes gh-4356
Checklist
New tests have been added to show the fix or feature worksIf needed, a docs issue/PR was created at https://github.com/jquery/api.jquery.com