WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 years ago

#31480 closed defect (bug) (fixed)

TinyMCE resize sandbox iFrame when images load

Reported by: mattheu Owned by: azaozz
Milestone: 4.2 Priority: normal
Severity: normal Version: 4.0
Component: TinyMCE Keywords: has-patch
Focuses: Cc:

Description

Possible bug - If an MCE View contains an image, and the view is sandboxed, the iFrame is not resized correctly to fit the image.

The reason for this is that the iFrame is resized using a MutationObserver. This is only triggered when a new element is inserted, or an attribute changes. But not when an image loads.

Fix - do resize on image load event. This could be done very simply like so:

$( iframeDoc.body ).find('img').load( resize );

However

  • This functionality would need to be duplicated inside the MutationObserver callback to handle any images that are added later.
  • I noticed that we can't rely on the MutationObserver callback alone because this is set up AFTER the iframe is created and the content added.

I think the solution here should be

  1. In the mutation observer calback we should check for new image elements, and trigger resize when that image loads.
  2. Set the mutation observer BEFORE the content is added to the iFrame so this gets triggered by the main preview body content. To do this the MutationObserver needs to observe iframeDoc and not iframeDoc.body

Any thoughts?

Attachments (6)

31480.diff (2.4 KB) - added by mattheu 5 years ago.
31480.2.diff (2.7 KB) - added by mattheu 5 years ago.
31480.patch (2.7 KB) - added by iseulde 4 years ago.
31480.2.patch (411 bytes) - added by azaozz 4 years ago.
31480-2.diff (424 bytes) - added by mattheu 4 years ago.
31480.3.patch (380 bytes) - added by iseulde 4 years ago.

Download all attachments as: .zip

Change History (25)

@mattheu
5 years ago

#1 @iseulde
5 years ago

  • Version changed from trunk to 4.0

Thanks!

#2 @iseulde
5 years ago

  • Milestone changed from Awaiting Review to 4.2

I'll look into this too together with all the rest. :)

@mattheu
5 years ago

#3 @mattheu
5 years ago

Refreshed patch

#4 @iseulde
5 years ago

  • Keywords has-patch added

I first need to know why this happened: [29615] (the change to body). @wonderboymusic?

#5 @wonderboymusic
5 years ago

Post Formats - when you click one, it changes the TinyMCE body class, see 2013 theme

This ticket was mentioned in Slack in #core by drew. View the logs.


4 years ago

#7 @iseulde
4 years ago

@matheu We can't use foreach because IE < 9 does not support that. In any case while would be the fastest here. And resize; needs parentheses. :)

Here's something we can use to test:

views.register( 'img', {
	content: '<!--<script--><img src="http://deelay.me/200/s.w.org/about/images/logo-comparison.png">'
} );

@iseulde
4 years ago

#8 @iseulde
4 years ago

Does the last patch work for you?

#9 @mattheu
4 years ago

Works OK for me!

#10 @iseulde
4 years ago

@azaozz, how does this look?

#11 @azaozz
4 years ago

As far as I see we only need to add resize to the sandbox iframe window.onload.

What would be the user case to support resizing after the view has been fully loaded? Keep in mind that this will disrupt the users if they are typing under the view, especially if another image is loaded 30 sec. later, etc.

@azaozz
4 years ago

#12 @azaozz
4 years ago

31480.2.patch seems to work well here.

#13 @iseulde
4 years ago

Works for me if the images is not inserted later.

#14 @mattheu
4 years ago

Doesn't actually fix my issue, although it does work for the testcase provided by @iseulde.

My problems is actually with a local image that is loaded normally. I think the image has already loaded before you're attaching resize to the load event.

I think the simplest way to get around this is to call resize manually after creating the iFrame, as well as listening for the load event on the iFrame window. I'm adding another patch.

Do you think we should debounce these calls to resize?

@mattheu
4 years ago

#15 @iseulde
4 years ago

Do you think we should debounce these calls to resize?

The callback is only called once.

I think the image has already loaded before you're attaching resize to the load event.

Wouldn't moving it before iframeDoc.write work?

#16 @mattheu
4 years ago

Hmm - after testing some more - I think the fix by azaozz does work OK :)

@iseulde
4 years ago

#17 @iseulde
4 years ago

Maybe better to add it before writing the document?

#18 @mattheu
4 years ago

Maybe better to add it before writing the document?

Makes sense. Tested and works fine for me.

#19 @azaozz
4 years ago

  • Owner set to azaozz
  • Resolution set to fixed
  • Status changed from new to closed

In 32056:

TinyMCE wpView: resize sandbox iframes on load. Fixes a problem with resizing after all images are loaded.
Props iseulde, mattheu. Fixes #31480.

Note: See TracTickets for help on using tickets.