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

Content inside <template> breaks extending <body> element. #421

Closed
tomalec opened this issue Feb 14, 2014 · 11 comments · Fixed by googlearchive/ShadowDOM#496
Closed

Content inside <template> breaks extending <body> element. #421

tomalec opened this issue Feb 14, 2014 · 11 comments · Fixed by googlearchive/ShadowDOM#496
Assignees
Labels

Comments

@tomalec
Copy link
Contributor

tomalec commented Feb 14, 2014

I wrote a custom element that extends <body> (http://jsbin.com/vohoc/1/edit). It worked perfectly fine in both Canary and regular Chrome (http://jsbin.com/cepew/2/quiet).

But when I put anything into element's template:

<polymer-element name="x-foo" extends="body">
  <template>Something here breaks it.<content></content></template>

http://jsbin.com/cepew/3/quiet

It blows with a ton of Uncaught Error: Assertion failed errors. Two at the beginning at PointerGestureEvent.js:18, and throws one for every mouse move.
(Chrome 32.0.1700.107 m)

@sorvell
Copy link
Contributor

sorvell commented Feb 18, 2014

I don't have Chrome32 to test on but do yo mind verifying this repro's with
the latest polymer release? I don't see a problem on Safari, FF, Chrome 33,
or Chrome 34.

Thanks.

On Fri, Feb 14, 2014 at 4:57 AM, Tomek Wytrębowicz <notifications@github.com

wrote:

I wrote a custom element that extends (
http://jsbin.com/vohoc/1/edit). It worked perfectly fine in both Canary
and regular Chrome (http://jsbin.com/cepew/2/quiet).

But when I put anything into element's template:

Something here breaks it.

http://jsbin.com/cepew/3/quiet

It blows with a ton of Uncaught Error: Assertion failed errors. Two at
the beginning at PointerGestureEvent.js:18, and throws one for every mouse
move.
(Chrome 32.0.1700.107 m)


Reply to this email directly or view it on GitHubhttps://github.com//issues/421
.

@sorvell
Copy link
Contributor

sorvell commented Feb 18, 2014

Oh, and kudos for using a jsbin as an HTMLImport =)

On Tue, Feb 18, 2014 at 10:34 AM, Steve Orvell sorvell@google.com wrote:

I don't have Chrome32 to test on but do yo mind verifying this repro's
with the latest polymer release? I don't see a problem on Safari, FF,
Chrome 33, or Chrome 34.

Thanks.

On Fri, Feb 14, 2014 at 4:57 AM, Tomek Wytrębowicz <
notifications@github.com> wrote:

I wrote a custom element that extends (
http://jsbin.com/vohoc/1/edit). It worked perfectly fine in both Canary
and regular Chrome (http://jsbin.com/cepew/2/quiet).

But when I put anything into element's template:

Something here breaks it.

http://jsbin.com/cepew/3/quiet

It blows with a ton of Uncaught Error: Assertion failed errors. Two at
the beginning at PointerGestureEvent.js:18, and throws one for every mouse
move.
(Chrome 32.0.1700.107 m)


Reply to this email directly or view it on GitHubhttps://github.com//issues/421
.

@tomalec
Copy link
Contributor Author

tomalec commented Feb 18, 2014

:) Thanks.

I still face same issue on
Chrome 32 - http://imgur.com/kp55nrF
Firefox 26.0

@sorvell
Copy link
Contributor

sorvell commented Feb 20, 2014

I'm unable to reproduce this now (see below) on Chrome 32 (Version 32.0.1700.107 m). Sorry, but can you verify one more time?

The test is loading polymer directly via github. This, unfortunately (or not) makes it a moving target as this build is frequently updated.

If the problem persists, can you include any Chrome flags that are set (you can go to chrome://version and paste the output next to 'Command Line'). Thanks for your patience.

@tomalec
Copy link
Contributor Author

tomalec commented Mar 3, 2014

I have fixed Polymer version to latest baster hash http://jsbin.com/cepew/5/edit
I still face this issue once I open chrome devtools.

chrome://version

Google Chrome   33.0.1750.117 (Oficjalna wersja 252094) m
OS  Windows 
Blink   537.36 (@167220)
JavaScript  V8 3.23.17.13
Flash   12.0.0.70
Client  Mozilla/5.0 (Windows NT 6.2; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/33.0.1750.117 Safari/537.36
Command Line    "C:\Program Files (x86)\Google\Chrome\Application\chrome.exe" --no-startup-window --flag-switches-begin --flag-switches-end

@sorvell
Copy link
Contributor

sorvell commented Aug 11, 2014

This seems to work on Chrome now:

http://jsbin.com/beyuy/1/edit

@arv and @jmesserly It doesn't appear to work with the ShadowDOM polyfill.

@arv
Copy link
Contributor

arv commented Aug 12, 2014

I really do not see why this would be different than say a div. I think the first step would be to see if this repros in a pure ShadowDOM polyfill.

@jmesserly
Copy link
Contributor

ah, tracked it down. It breaks because this line:
https://github.com/Polymer/ShadowDOM/blob/master/src/ShadowRenderer.js#L76
expects to be operating on the unwrapped node. But when it calls insertBefore, it gets the overridden method due to https://github.com/Polymer/ShadowDOM/blob/master/src/wrappers/Document.js#L234
and that expects wrappers (https://github.com/Polymer/ShadowDOM/blob/master/src/wrappers/Node.js#L369)

Not sure the best fix. We could capture the native insertBefore method as "originalInsertBefore" and call that directly from ShadowRenderer to ensure we're getting the native one. Not sure if there's any other way to reliably get the native (visual) methods for "body", since they're all overridden.

Thoughts?

@jmesserly
Copy link
Contributor

By the way, here's a 1-page repro: http://jsbin.com/bitebohovezo/1/edit
It works if you s/body/div/

@arv
Copy link
Contributor

arv commented Aug 13, 2014

Storing the original function is what we had to do in a few other places.

@jmesserly jmesserly added the p1 label Aug 14, 2014
@jmesserly
Copy link
Contributor

simplified the repro a bit further (just platform.js now): http://jsbin.com/bitebohovezo/2/edit
working on a test case + fix

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

Successfully merging a pull request may close this issue.

4 participants