Copy paste a long shortcode wpview (video, gallery... ) is broken in Firefox

[bduclos]

Steps to reproduce:

• In firefox, copy the following code

  [video width="640" height="480" mp4="https://videos.files.wordpress.com/GbdhpGF3/video-25313e7f01_hd.mp4" poster="https://i2.wp.com/videos.files.wordpress.com/GbdhpGF3/video-25313e7f01_scruberthumbnail_0.jpg"][/video]
  

• In Visual Mode, select the video view, copy it (Ctrl+c) and paste it (Ctrl+v) below. Switch to Text mode. Multiple newlines are added which results in the shortcode being broken on both back-end and front-end.

  [video width="640" height="480" 
  mp4="https://videos.files.wordpress.com/GbdhpGF3/video-25313e7f01_hd.mp4"
  
  poster="https://i2.wp.com/videos.files.wordpress.com/GbdhpGF3/video-25313e7f01_scruberthumbnail_0.jpg"][/video]
  

The same happens with the gallery shortcode or any wpview shortcode where the shortcode text is a bit long.
(e.g. [gallery link="file" columns="5" size="large" ids="495,499,498,533,664,638,577,575,497,773,943,670,668,496,494,487,479,673,675,676" orderby="rand"] )

[azaozz]

Can't reproduce in Firefox on Win10 (or any other browser). Tested with the above example in trunk and 4.6. Looks like "something" is adding line breaks before shortcode attributes on purpose. Could this be caused by a plugin? Or perhaps there were some "invisible" characters in the shortcodes when they were pasted in the editor for the first time that aren't in the example above?

[bduclos]

On both a fresh WP trunk install and a fresh WP 4.6.1 install (no plugins), on Win10 and Firefox latest version (49.0.2), retested with the 2 following methods but still having the same issue:

• Paste the following video shortcode in Text mode and copy paste the wpview in Visual mode.

[video width="640" height="480" mp4="https://videos.files.wordpress.com/GbdhpGF3/video-25313e7f01_hd.mp4" poster="https://i2.wp.com/videos.files.wordpress.com/GbdhpGF3/video-25313e7f01_scruberthumbnail_0.jpg"][/video]

• In Visual Mode, click on Add Media, create a gallery with a lot of images and set its settings (Link To, Columns, Random order, Size) different than the default ones in order to generate a long shortcode string. Insert the gallery, copy and paste the wpview.

[iseulde]

I can reproduce this now in Firefox. It's breaking the line at certain points such as space, dot, dash etc.

[iseulde]

Actually I can't reproduce entirely. Just after switching to text and back the blocks are blank.

[lukecavanagh]

@bduclos

I can reproduce a similar issue in Firefox Developer 49.0a2 using WP 4.6.1.

[iseulde]

This is a really strange issue... Scripts are run at first (so the contents set), but then shortly afterwards the whole iframe unloads. Try the patch above and you'll see it "test"in the console, then "unload". I've been looking for hours but couldn't find the cause so far.

[iseulde]

I found a link! If editor scrolling is disabled, all works well.

[gitlost]

This is a really strange issue... Scripts are run at first (so the contents set), but then shortly afterwards the whole iframe unloads.

This is caused by the quirks routine refreshContentEditable() which (Firefox-only) reloads the body if there's no selection. Calling this early (so that later calls are a noop) is one way to workaround this issue.

The OP is caused by Firefox prettyprinting text/html into the clipboard, wrapping long lines by inserting newlines. Shortcodes with attributes then get wpautoped causing them not to parse. The following patch uses the existing pastepreprocess action in "wpviews/plugin.js" to remove them.

[gitlost]

Remove newlines in pastepreprocess and workaround refreshContentEditable.

[iseulde]

@gitlost, that looks right.

Test code (see ​https://github.com/tinymce/tinymce/blob/master/js/tinymce/classes/util/Quirks.js):

(function() {
  var body, parent;
  body = tinyMCE.activeEditor.getBody();
  parent = body.parentNode;
  parent.removeChild(body);
  parent.appendChild(body);
  body.focus();
})()

If you run this the iframes will be empty.

[iseulde]

I'd prefer fixing this so that whenever the whole content reloads by something, the iframes contents are reloaded as well.

We can still trigger the quirks function beforehand though to prevent excessive reloading. Instead of running editor.quirks.refreshContentEditable() we can run editor.focus() which runs the former internally.

[iseulde]

Added a new patch that also removes the timeout that was once added to fix this issue. See [29513].

[iseulde]

Finding more bugs and mistakes while doing this.

Patch 6:

• Set focus before rendering.
• Empty views on hide. Missed in #36434.
• Fix argument in wp.mce.views.render.
• Remove timeout added in [29513].
• Rerender views if they are unloaded.

[azaozz]

Replying to iseulde:

38511.6.patch looks good. In 38511.7.patch also added the removal of line breaks from shortcodes from 38511.4.patch (props @gitlost). Moved it to createInstance() as discussed in Slack, but it makes sense to try to fix that in Firefox for all pasted content. Not sure if we should do that, probably better to be handled upstream in the paste plugin.

Also noticing (non-fatal) mediaelement.js errors in Firefox and IE11 when switching to the Text editor while a movie is playing. Seems pausing the player and removal of player instances fails in these browsers. This is not caused by the above patch.

[bduclos]

Other bug: when you copy paste a view, the toolbar with the edit/delete buttons appears above the pasted view while the view is not selected (clicking on the buttons does nothing). Bug on all browsers.

[iseulde]

Normally we'll never have more than one shortcode in createInstance, because they are already matched, so I'm not sure if the extra loop is necessary. Also, I don't think it would be bad to disallow line breaks in general in these view texts as it will lead to nothing but problems.

Ideally this is handled by mce indeed, but it doesn't seem like we can wait for that. Let's fix it like this for now.

[iseuldebot]

​Compare

[iseulde]

In 39282:

TinyMCE views: fix Firefox issues.

• Set focus before rendering to prevent reload in Firefox.
• Rerender views if they are unloaded.
• Remove timeout added in [29513].
• Fix argument in wp.mce.views.render.
• Empty views on hide. Missed in #36434.

Props gitlost, azaozz, iseulde.
Fixes #38511.

[iseulde]

@bduclos Could you create a now ticket for the other bug?

[bduclos]

Replying to iseulde:

@bduclos Could you create a now ticket for the other bug?

#38849

[iseulde]

The commit here causes the page to automatically scroll down when it loads.

[iseulde]

Patch only sets focus when new content is loaded in the editor, and it's not the initial load. That happens when switching from text to visual.

[iseulde]

In 39299:

TinyMCE: fix automatic scroll on page load.

Set focus when new content is loaded in the editor, but not when it's the initial load.

Fixes #38511.

[azaozz]

Replying to iseulde:

Yeah, we've had problems with calling editor.focus() before. It behaves quite differently in different browsers when called on load or during load.

How about we go back to editor.quirks.refreshContentEditable();? I know it used to be a private function, but seems it is not any more (the leading underscore was removed): ​https://github.com/tinymce/tinymce/commit/b4576a4c1f96801241c710ad51dfacc605d24e1f.

It is better for this as it only runs in Firefox and only when needed/there is no selection.

[azaozz]

In 39334:

TinyMCE: avoid calling editor.focus() on loading the content in the editor. It may trigger scroll-into-view in the browser. Call the quirks fix in TinyMCE directly.

Props gitlost.
Fixes #38511.

[westonruter]

In #55491 we're working to remove the deprecated unload event handler from core. One of the instances was added in this ticket. I tried to reproduce the issue in Firefox today (with the Classic Editor plugin enabled), and I was unable when the reload code from r39282 is removed. I take that to mean the Firefox-specific issue was fixed in the past 7 years. So I'm recommending deleting that code. Nevertheless, if anyone knows of a good reason for it to remain, please chime in on #55491.

[westonruter]

In 56809:

Administration: Remove deprecated unload event handlers and use pagehide (and pageshow) when appropriate.

Use pagehide event instead of unload in the following cases:

• For classic editor to release the post lock.
• In Text widget to rebuild editor after dragging widget to new location in classic widgets interface.
• To clear out the window.name when navigating away from a post preview.
• To suspend heartbeat, while also using pageshow event to resume as if it had been a focused tab in case page restored from bfcache.

Also:

• Remove obsolete mobile cleanup code in js/_enqueues/lib/gallery.js (introduced in [9894]). Do same for src/js/_enqueues/wp/media/models.js (introduced in [22872]). See #22552.
• Remove obsolete Firefox-specific workaround in js/_enqueues/wp/mce-view.js from [39282]. See #38511.

Fixes #55491.
Props spenserhale, westonruter, adamsilverstein, azaozz, shawfactor, peterwilsoncc, swissspidy.