-
-
Notifications
You must be signed in to change notification settings - Fork 623
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
Many aurelia libraries are broken in iOS 16 #1003
Comments
I'm not able to guess what has started to fail. |
I thought the same (that this is not specific to Aurelia) and I tried to find any issues with Safari 16 on other frameworks or whatever, but I couldn't find anything. You can see this repo if you like to test anything. |
I also have issues with Safari 16 too, but I'm not sure it's the same reason. Here is my repo (close to jded76 repo but with Fast) and the bug in a nice animation : How can I help to find a solution??? |
Thanks @Tamu, nice 🙂 |
Awesome @bigopon, if I add
On the other hand if I add
|
@Tamu can you help check the console to see if there is any errors for the one without if? Please paste the whole trace if possible. |
here is all the console log :
|
I confirm that there is a new issue with iOS 16 when observing properties in Aurelia 1. The In the file aurelia-fast-adapter.ts, in iOS 16 the Any help on this matter is very appreciated. |
I searched further on the problem, and if I'm right (because I don't know the internals of aurelia) the problem is not at the detached event, but at the creation of the View. I had a breakpoint at this this line (aurelia-templating - ViewFactory - create), hit refresh on the browser and at the first hit of the breakpoint I saw that on Chrome the template has a valid document but on Safari it has an about:blank document (see attached pictures). @bigopon do you have any idea? Am I on the right track? |
@jded76 |
@jded76 right, thanks. I guess you have replicated the bug. Safari 16 |
A test can be like this I think const div = document.createElement('div')
div.innerHTML = '<template><button>';
const fragment = div.firstChild.content;
console.assert(fragment.ownerDocument === null)
document.adoptNode(fragment);
console.assert(fragment.ownerDocument === document); With Safari 16, it'll fail while it'll succeed with Safari < 16 |
I just found that the source has the right document, but the source.content has the wrong one at this line. |
That is expected, before |
Sorry this is the same behavior as in Chrome. Ignore my previous comment. |
Actually the ownerDocument is not null, but an empty document (about:blank). What can we do about that from here? |
Few investigations: For reference, I am able to bypass this issue if I manually modify the ViewSlot removeAll (this issue only used removeAll code path) method, to do detached() callback before child.removeNodes(). In other words, I tested calling detached() callback before the view is removed from DOM. So it's kind of "detaching" callback like au2, and it worked. That confirmed Safari 16 has issue in dealing with detached element.
|
Thanks @3cp |
I'm still thinking about this.
Safari is the first major browser that supports that rule. It look's like that this is not a bug of Safari, but a feature and we must adapt to overcome the problems. On the other hand I remember that I tested the detach event of aurelia-kendoui-bridge without navigating to other page and it didn't have the same problem. I'm still searching on this... |
Finally I'm getting somewhere for aurelia-kendoui-bridge, maybe for other libraries too that using jquery. I changed this line from Edit: |
One not-so-great way to partially ignore the error. Only works for route component. <router-view swap-order="before"></router-view> This swap order loads new route component before unloading old one. The exception still happens on the old component, error log still shows up in console, but it would not stop the new route from loading up. |
@bigopon, since @jded76 found out Safari is likely following the updated spec. There is no much we can do about it. I tested replace the adoptNode with importNode, it works. //content = DOM.adoptNode(source.content);
content = document.importNode(source.content, true); Obvious the deep clone has performance penalty, but it seems necessary to be compatible with updated spec. The other option is not using document fragment at all, but I don't think that's possible for au1 since au1 uses native browser html parser. Regarding the spec, I have trouble to follow "a DocumentFragment node whose host is non-null", any idea what's the api to inspect the host of a fragment? |
I'm not sure that aurelia will have a problem with the change of the spec. You can see (with Safari 16+) that the template and the template.content have an empty ownerDocument, and I think this is right because they don't belong to the main document. The only issue I can see it's the lack of the "detaching" event.
From the commit of Safari, they check if the node is type of TemplateContentDocumentFragment. |
@bigopon I recall you have some benchmark setup? if |
If this is right, then if we change to |
And I think if we change the remove process to just remove the elements from the current view (and not adding them back to the template), then we will have the same problem, that the elements don't have an ownerDocument Or maybe even worse, that the elements don't exist any more on the detached event? |
https://bugs.chromium.org/p/chromium/issues/detail?id=1031811 It seems both Chrome and Firefox are still waiting for some further spec change. whatwg/dom#819 There is new option forceDocumentFragmentAdoption mentioned in the pending change. |
From my limited understanding, au1's remove process only returns view to a cache (which is just a simple JS array to hold those views). |
Also, it's little bit unusual for DOM spec to introduce a breaking change. |
I'm not sure but when I was searching, I have shown this and I thought that is the removal process.
I have shown that too, the request of the change is two years old but I couldn't follow them... |
I think you are right :-) I didn't read these lines. |
webkit team is doing something on that. I don't quite understand the details, but it seems they think the bug I raised is a valid bug. 🤞🏻 |
Thanks @3cp @jded76 🤞 . We have a way around it, which is around the template parsing time: immediately import the fragment after creating the template from a string. And perf won't be an issue here since we are never going to compile 10000 times per second. |
Too bad, webkit closed the ticket as "intentional behaviour" :-( |
It looks like that the change of the specs is stuck without any updates and we have to figure out what we have do to be compatible with Safari again... |
Maybe we can have a snippet to let apps temporary patch it until further action from the spec and Safari? Then apps can use it to verify whether it's a bug from this change or not. I'm a bit afraid of doing any "fix" for this right now, before a resolution in the spec. |
I agree with that, that's why I asked your opinion if this affects the core functionality. About the snippet and the patch, i can't follow you. What do you mean? |
Has anyone found a temporary work-around for this? Dealing with this issue in production still. |
We solved our production problems by removing some incompatible libraries and for aurelia-kendoui-bridge we changed something in jquery and it's working again. |
good news, it's being revised https://bugs.webkit.org/show_bug.cgi?id=246899 |
PR is up, so I guess we'll have some good updates for our customers/users soon. |
@jded76 I don't see this problem anymore on Safari v16.6. |
I think it's working again. |
@bigopon pls close it :-) |
I'm submitting a bug report
aurelia-bootstrapper: 2.3.3
Please tell us about your environment:
Operating System:
OSX 16
Node Version:
14.16.0
6.14.11
CLI 1.3.1 | webpack 4.46.0
Browser:
iOS 16 Safari
Language:
all
Current behavior:
After the new IOS 16, Safari has a weird behavior in Aurelia, causing many libraries to fail at the detached event.
After some searching I found that the ownerdocument of the element is an empty one (about:blank) and many libraries that do cleanup at the detached event are getting errors. More details can be found here .
There is also a repro on the issue 808 of aurelia-kendoui-bridge
aurelia-ui-toolkits/aurelia-kendoui-bridge#808
Expected/desired behavior:
Be compatible with Safari.
Can we do something to be compatible again with the new Safari?
The text was updated successfully, but these errors were encountered: