WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 6 years ago

#30646 closed defect (bug) (fixed)

oEmbed preview loading does not trigger editor scrolling

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

Description

You can't scroll to see the full preview until you move the cursor or something.

Attachments (2)

oembed-loading-no-scrolling.gif (920.4 KB) - added by celloexpressions 7 years ago.
30646.patch (783 bytes) - added by iseulde 6 years ago.

Download all attachments as: .zip

Change History (8)

@iseulde
6 years ago

#1 @iseulde
6 years ago

  • Component changed from Editor to TinyMCE
  • Focuses javascript added
  • Keywords has-patch added; needs-patch removed
  • Milestone changed from Awaiting Review to 4.2
  • Severity changed from normal to minor

#2 @azaozz
6 years ago

@iseulde not sure if we should call editor.nodeChanged() on iframe resize. The node hasn't really been changed :) Maybe fire a wpAdjustSize or editorSizeChanged event and listen for it from the wpautoresize plugin?

Last edited 6 years ago by azaozz (previous) (diff)

#3 @iseulde
6 years ago

Yeah, maybe that's better. Just used that because it's mainly used for UI updates.
https://github.com/tinymce/tinymce/blob/master/js/tinymce/classes/Editor.js#L1198

#4 @iseulde
6 years ago

Hm, I still think we should use nodeChanged because if you start adding inline toolbars their position should update. And that event is used for UI updates.

#5 @azaozz
6 years ago

Right, nodeChanged is used for UI updates but also for selection/caret range updates (most notably in IE). My concern is that the iframe resize function is fired with mutationObserver that may trigger several times in a very short time while we are trying to use/update the same selection/caret range. This could result in race condition.

On the other hand comparing the iframe height to the iframe's body height before resizing reduces the number of times we fire nodeChanged, so it shouldn't fire excessively. Good way to test this is by embedding a "twit", fires only two times here.

Lets add it and see if there will be any problems in IE.

#6 @azaozz
6 years ago

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

In 31466:

TinyMCE wpView: fire nodeChanged when an embedded iframe is resized so we can adjust the editor height and other UI components. Props iseulde, fixes #30646.

Note: See TracTickets for help on using tickets.