Skip to content
This repository has been archived by the owner on May 26, 2022. It is now read-only.

Guard against non-existing iframes in resize. #9

Closed
kjarmicki opened this issue Nov 19, 2014 · 4 comments
Closed

Guard against non-existing iframes in resize. #9

kjarmicki opened this issue Nov 19, 2014 · 4 comments

Comments

@kjarmicki
Copy link
Contributor

Sometimes there are cases when one plugin is removing an iframe and another, called directly after, tries to resize it, which causes errors in Iframe.resize() method (trying to get to style property of element that no longer exists).

This can be guarded against by wrapping styling in a conditional, something like:

if(this.element && this.element.style) {
    this.element.style.width = validSize(this.width);
    this.element.style.height = validSize(this.height);
}

It could also be dealt with in every plugin individually, but I think that would be redundant.

@kjarmicki kjarmicki mentioned this issue Nov 24, 2014
sveisvei added a commit that referenced this issue Nov 24, 2014
Issue #9. Guard against non-existing iframes in resize.
@sveisvei
Copy link
Contributor

Merged, seamed sensible to have a element check.

But should we maybe look into if a removal of the iframe could "stop" the plugin-call-chain?
We could e.g. not allow plugins to remove the iframe directly, but via a remove() on the manager - that correctly "shuts it down".

@kjarmicki
Copy link
Contributor Author

Yes, that would be a nice enhancement. We still need to check existence of element in resize() tho, because there are cases that this metod could be called outside the plugin.
Take Manager.render() for example, it executes a callback when iframe is rendered. This callback recieves item as an argument and can be called with timeout, keeping item in closure. So there's a possibility that when timeout ends and callback code executes item.iframe.resize(), the actual iframe is gone and that would throw an error.

@sveisvei sveisvei removed the bug label Jun 5, 2015
@sveisvei
Copy link
Contributor

sveisvei commented Jul 2, 2015

Next release brings a cleanup and rerender setup https://github.com/gardr/host/blob/master/lib/manager.js#L344

@sveisvei sveisvei closed this as completed Jul 2, 2015
@gregersrygg
Copy link
Member

@sveisvei Your suggestion about instructing manager to remove an item is interesting. Take a look at the discussion in #25. Maybe we should have a way for plugins to send commands to host/ext?

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

No branches or pull requests

3 participants