Skip to content

Dialog: Fix resizing with iframes #1214

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

Closed
wants to merge 1 commit into from
Closed

Conversation

Rantanen
Copy link

Make sure the element that overlays iframes during dialog resize follows
the iframe size during the resize operation.

Fixes #9919

Quick fix for the iframe resizing issue. Few things that bother me though:

The iframe the div is overlaying is stored using $.data. Is there a better way and do we need to clear this reference in the _unblockFrames or is it enough to just delete the div itself?

Performance-wise we shouldn't need to iterate through all the iframes in _resizeBlocks. It should be enough to iterate through the iframes inside the current dialog as those are the only ones likely to change size. What would be the cleanest way to implement this? Storing them in different field in _blockFrames, $(this).children().filter( this.iframeBlocks ), giving them a className and using $(this).children("...") or something else?

Make sure the element that overlays iframes during dialog resize follows
the iframe size during the resize operation.

Fixes #9919
@Rantanen
Copy link
Author

Tested in up to date Chrome, Firefox and IE11. IE11 was a bit slow in the resize but I don't believe this was a result of the fix as the test case had pretty simple HTML. Didn't compare IE11 performance with and without the fix applied.

@scottgonzalez
Copy link
Member

I hate to say this, but given how long this problem has existed, I'm inclined to just leave it until we can address this with the interaction rewrite. Same concern as I expressed in #1215 (comment).

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

Successfully merging this pull request may close these issues.

2 participants