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

[0.8] x-repeat failing to stamp instances on safari #1443

Closed
nevir opened this issue Apr 23, 2015 · 8 comments
Closed

[0.8] x-repeat failing to stamp instances on safari #1443

nevir opened this issue Apr 23, 2015 · 8 comments

Comments

@nevir
Copy link
Contributor

nevir commented Apr 23, 2015

I've gone pretty far down the rabbit hole, but I think it's time to pass off to you @kevinpschaaf

To see the weirdness:

Notice that:

  • After setting the items, the template properly debounces and calls render.
  • Inside render, it is properly calling _insertRow -> _generateRow -> stamp
  • The result of a stamp is a completely busted row (missing node/fragment, no children)
@nevir nevir changed the title [0.8] x-repeat failing to stamp instances on safari (sometimes) [0.8] x-repeat failing to stamp instances on safari Apr 23, 2015
@kevinpschaaf
Copy link
Member

Looks like a safari native template bug with importing template content containing nested templates... nested templates lose their content.

http://jsbin.com/siliwarete/1/edit?html,console

For all polymer templates, the annotator recursively scans template contents and removes the content fragment from nested templates altogether, and hands it back (as _content) as part of the configuration lifecycle after stamping, which avoids this problem. This is done for several reasons: a.) performance (cloneNode no longer needs to clone lots of nested template contents each time an outer template is stamped, b.) avoids an old bug in Chrome which results in nested template contents being incorrectly upgraded, and c.) apparently avoids this Safari bug. :)

As a short-term workaround, you could probably use a <template is="x-if" restamp> in place of the vanilla template, and toggle if to restamp, since that comes with the nested template handling and gives you a sort-of convenient stamp/restamp API. Doing the template handling yourself is tricky, because x-repeat assumes it will have its _content handed back at ready() time, and you don't have an easy way of getting it there if you don't participate in Polymer's ready/configuration cycle.

Do you want to move this issue to https://github.com/PolymerElements/test-fixture and decide how to handle it there?

@nevir
Copy link
Contributor Author

nevir commented Apr 24, 2015

Wow that's a hell of a bug. Thanks for tracking it down!

Yeah, it doesn't look like there's much we can do on the Polymer side here (except maybe warn when encountering it)

@IntranetFactory
Copy link

I'd also faced the Safari (and IE) problem with nested templates and preserve-content. I tried to workaround the issue by copying template node.content to dom node._content after importNode in instanceTemplate (see https://github.com/TangereJs/Tangere/blob/0.8-preview/Tangere.html)

That fixed my use case - but I've no idea if that could also fix for this issue.

@kevinpschaaf
Copy link
Member

@IntranetFactory What problem was that solving? Seems that moving content to _content after importing won't solve the Safari bug, since that's too late (importNode is what breaks the nested template's content)... Did I miss something, or was this fixing something else you ran into?

@IntranetFactory
Copy link

In my use case the .content of nested template nodes, marked with preserve-content are empty in Safari and IE. In Chrome and FF .content contains the template.

The .content was "lost" in document.importNode. In template .content exists, but in the dom variable it was missing only in Safari and IE. In Safari I assumed it's known bug of nested templates, and in IE probably the polyfill fails. It seemed like importNode "forgets" to copy .content when creating the document fragment.

So my workaround is to copy the original template.content to the imported dom created by document.importNode.

So instanceTemplate then returns all template nodes with .content populated in all browsers.

@kevinpschaaf
Copy link
Member

Ah, got it.

@kevinpschaaf
Copy link
Member

Issue should be moved to text-fixture and that element will need to deal with the Safari issue. It could leverage <template is="dom-if" restamp>, or re-do some of the work already done by the Polymer annotator/templatizer logic.

@tjsavage
Copy link
Contributor

auto-moving this issue to PolymerElements/test-fixture#8 and closing this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants