Skip to content
This repository has been archived by the owner on Mar 13, 2018. It is now read-only.

Wrap the document, and handle null #132

Merged
merged 1 commit into from
Oct 1, 2014

Conversation

jakemac53
Copy link
Contributor

No description provided.

@jakemac53
Copy link
Contributor Author

@jmesserly I tested doing this as you suggested yesterday and it makes all the tests pass, ptal.

@jmesserly
Copy link
Contributor

Ah, I was thinking to do it on the Dart side (web_components package). There we know platform.js is included and we have ShadowDOM. We don't make that assumption here.

Last I heard @sorvell is working on a JS fix. I offered to help but he seemed eager to do it :)

@sorvell
Copy link
Contributor

sorvell commented Oct 1, 2014

lgtm. I was hoping to wrap (hah!) this into some other refactoring I want to do but it's going to take me a bit longer. This fix seems fine for now.

sorvell pushed a commit that referenced this pull request Oct 1, 2014
Wrap the document, and handle null
@sorvell sorvell merged commit efdbbcb into googlearchive:master Oct 1, 2014
@jmesserly
Copy link
Contributor

Do we have any concerns about x-tags or other users that consume CustomElements without ShadowDOM polyfill?

@jmesserly
Copy link
Contributor

(Actually even SD polyfill doesn't put wrapIfNeeded on window. That must come from platform.JS)

@sorvell
Copy link
Contributor

sorvell commented Oct 1, 2014

There's direct support in the CE polyfill for this (
https://github.com/Polymer/CustomElements/blob/master/src/Observer.js#L220
).

On Wed, Oct 1, 2014 at 10:13 AM, John Messerly [email protected]
wrote:

(Actually even SD polyfill doesn't put wrapIfNeeded on window. That must
come from platform.JS)


Reply to this email directly or view it on GitHub
#132 (comment)
.

@jakemac53 jakemac53 deleted the patch-1 branch October 1, 2014 20:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants