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

TypeError: observerHandle is null in Firefox #269

Open
1 of 8 tasks
TrebuhD opened this issue Mar 2, 2018 · 13 comments
Open
1 of 8 tasks

TypeError: observerHandle is null in Firefox #269

TrebuhD opened this issue Mar 2, 2018 · 13 comments

Comments

@TrebuhD
Copy link

TrebuhD commented Mar 2, 2018

Description

On firefox (v58.02), I get the following error after I destroy one particular element which uses iron-overlay-behavior (when navigating to a different route):

TypeError: observerHandle is null

The element in question contains a list of elements, each of them using paper-dropdown-menu. When I delete the paper-dropdown-menu, everything works well. If I include paper-dropdown-menu, even one without any content, it throws this error.

The error is particularly bad, because after it occurs, half of the website refuses to work.

The error seems to be coming from this line:

Polymer.dom(this).unobserveNodes(this._observer);

I'm using webcomponents-lite.js.

Expected outcome

No error is thrown

Actual outcome

Error thrown: TypeError: observerHandle is null

Live Demo & Steps to reproduce

Go to this page, click the downwards facing arrow on the bottom right and then click the green "Pali" button on the left of the element that opened up. The error will occur.

Here's the code for the item containing the dropdown (sc-language-menu on line 200).

Browsers Affected

  • Chrome
  • Firefox
  • Safari 9
  • Safari 8
  • Safari 7
  • Edge
  • IE 11
  • IE 10

Stack trace from the error:

unobserveNodes@http://localhost/bower_components/polymer/lib/legacy/polymer.dom.html.js:59:7
detached@http://localhost/bower_components/iron-overlay-behavior/iron-overlay-behavior.html.js:196:7
detached@http://localhost/bower_components/polymer/lib/legacy/class.html.js:242:11
detached@http://localhost/bower_components/polymer/lib/legacy/class.html.js:240:9
detached@http://localhost/bower_components/polymer/lib/legacy/class.html.js:240:9
detached@http://localhost/bower_components/polymer/lib/legacy/class.html.js:240:9
disconnectedCallback@http://localhost/bower_components/polymer/lib/legacy/legacy-element-mixin.html.js:106:9
Fd.prototype.disconnectedCallback@http://localhost/bower_components/webcomponentsjs/webcomponents-lite.js:136:317
O@http://localhost/bower_components/webcomponentsjs/webcomponents-lite.js:132:114
we/<@http://localhost/bower_components/webcomponentsjs/webcomponents-lite.js:147:366
__detachInstance@http://localhost/bower_components/polymer/lib/elements/dom-repeat.html.js:576:9
disconnectedCallback@http://localhost/bower_components/polymer/lib/elements/dom-repeat.html.js:300:9
Fd.prototype.disconnectedCallback@http://localhost/bower_components/webcomponentsjs/webcomponents-lite.js:136:317
O@http://localhost/bower_components/webcomponentsjs/webcomponents-lite.js:132:114
we/<@http://localhost/bower_components/webcomponentsjs/webcomponents-lite.js:148:95
__teardownInstance@http://localhost/bower_components/polymer/lib/elements/dom-if.html.js:243:13
__render@http://localhost/bower_components/polymer/lib/elements/dom-if.html.js:147:9
__debounceRender/this.__renderDebouncer<@http://localhost/bower_components/polymer/lib/elements/dom-if.html.js:102:114
setConfig/this._timer<@http://localhost/bower_components/polymer/lib/utils/debounce.html.js:30:9
microtaskFlush@http://localhost/bower_components/polymer/lib/utils/async.html.js:21:11
TrebuhD added a commit to TrebuhD/iron-overlay-behavior that referenced this issue Mar 2, 2018
@valdrinkoshi
Copy link
Member

Hi @TrebuhD, I couldn't reproduce the issue through the website, it seems the content is not being served right now. Would you mind providing a smaller repro through a jsbin? e.g. http://jsbin.com/fowahen/4/edit?html,output

Regarding the bug, if this._observer is null it means attached was never invoked, which means there is a bug in polymer itself, e.g. Polymer/polymer#4550 or Polymer/polymer#4049 seem to be related.

@TrebuhD
Copy link
Author

TrebuhD commented Mar 3, 2018

Hi @valdrinkoshi . I don't know if I'll be able to produce a jsbin repro, as I don't know what exactly causes this error. I use the exact same elements in other places in the code and it works fine there.

The site is online now though! We had a small bug on production. You can check out the error over there as described in my original post.

I suspect it's a polyfill issue and not a Polymer issue itself - it doesn't happen in Chrome or any browser that has native Shadow DOM/Custom Elements implemented. In any case, I only got this error with iron-overlay-behavior inside paper-dropdown-menu and just in this one instance and my PR fixes it.

@TrebuhD
Copy link
Author

TrebuhD commented Mar 5, 2018

Just to let you know, I'll soon be swapping the deployed site to a working version without the error (where I manually edited the line in my local iron-overlay-behavior), so you won't be able to reproduce the error over there anymore. Hopefully, if we could merge that PR, I can avoid creating a fork of iron-overlay-behavior and continue using this repository.

@valdrinkoshi
Copy link
Member

The website has minified code, and it's hard to add debugging points where I'd want. Can you provide a smaller repro? Skimming through the codebase, I see that there are several nested dom-repeats and dom-ifs, e.g there are 2 nested ifs, here and here.
We cannot merge code without knowing the root cause. Also, each fix requires a unit test to avoid future regressions, so having a smaller repro is needed before we can proceed further.

@TrebuhD
Copy link
Author

TrebuhD commented Mar 5, 2018

Thanks for your answer. I'll try to provide a repro soon.

BTW, are nested dom-ifs a bad thing? I had no idea.

@valdrinkoshi
Copy link
Member

No it's not bad, it's just hard to debug from my side :) I tried following the data flow but got easily lost, and couldn't easily add breakpoints in the places I wanted to in the minified code :/

@snovikov-gd
Copy link

snovikov-gd commented Apr 2, 2018

FYI guys, I've faced with the same issue in Edge 16. I've fixed it the same way @TrebuhD is suggesting and have to use a fork of this repo.
Unfortunately, I cannot provide repro. I can just say this bug is floating in Edge, sometimes it does not appear in the same conditions. I didn't manage to understand when exactly, but F12 tools opened on the Network tab magically increases possibility it won't appear.

Here you can see there is no '_observer':
observer_is_null

And here it fails:
observer_is_null_2

It would be great to have this fix merged.

@valdrinkoshi
Copy link
Member

This looks very much related to a bug in ShadyDOM where listeners weren't actually removed webcomponents/shadydom#190. This was fixed in shadydom v1.0.13, included in webcomponentsjs v1.1.1 - can you confirm you're using either shadydom v1.0.13 or webcomponents v1.1.1?

@synopticum
Copy link

@valdrinkoshi I'm using v1.1.1. I'm sorry, I found a piece of code that manually changes this._observer in runtime so the above is likely related to our codebase, not iron-overlay-behavior component itself.

It would be convenient to have if (this._observer) check though.

@fgirardey
Copy link

I'm actually facing the same issue, you can reproduce this bug by forcing polyfills on Chrome.

<script>
    if (window.customElements) window.customElements.forcePolyfill = true;
    ShadyDOM = {force: true};
</script>
<script type="text/javascript" src="webcomponentsjs/webcomponents-loader.js"></script>

I'm actually forced to override the IronOverlayBehaviorImpl.detached method with the same fix than @TrebuhD has commited.

Why waiting so long to release the fix?

@rpapeters
Copy link

I'm also having this issue in (at least) FireFox and created a patch on the Polymer.DomApi.prototype.unobserveNodes function like this:

Polymer.DomApi.prototype.unobserveNodes = function(observerHandle) { observerHandle && observerHandle.disconnect(); }

So the difference is just checking if the observerHandle has a value before calling the disconnect function on it. For me this fixes the issue, what could go wrong here?

@aletorrado
Copy link

+1 !!!

@aletorrado
Copy link

Any update on this?

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

No branches or pull requests

7 participants