Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#42553 closed defect (bug) (fixed)

IE11: content loss when editing a post (Object doesn't support property or method 'matches')

Reported by: afercia's profile afercia Owned by: westonruter's profile westonruter
Milestone: 4.9 Priority: high
Severity: critical Version: 4.9
Component: Editor Keywords: has-screenshots has-patch dev-reviewed commit
Focuses: Cc:

Description

Reproduced on 3 different Windows machines thanks to @SergeyBiryukov.

Probably related to the changes to keep the selection when switching editor mode, see [41630] and following changes. See also #42530 and #42531.

To reproduce:
Create a new post with some simple content, for example:

Welcome to WordPress. This is your first post. Edit or delete it, then start writing!

Second paragraph.

Third paragraph.
  • publish the post and navigate away from the page
  • open again the post, make sure the Editor is in "Visual mode" on first load
  • switch to "Text mode"
  • switch back to "Visual mode"
  • the first paragraph in the content gets removed

Keep switching editor modes: I've observed also the second and third paragraphs get removed or, depending on if there's a selection or the caret position, parts of them but couldn't reproduce consistently.

I'm not sure it's really related, but the error Object doesn't support property or method 'matches' happens when switching Editor mode and comes from TinyMCE, not sure what code in the keep selection feature is using TinyMCE methods.

Also, from @SergeyBiryukov:

I can also reproduce the Object doesn't support property or method 'matches' error even without creating a post:

  1. Open "Add New Post" screen.
  2. Make sure Visual tab is displayed.
  3. Click anywhere inside the editor content area and you'll get the error.

https://cldup.com/5tX-oDSx_A.png

Attachments (9)

VirtualBox_IE11 - Win7_15_11_2017_07_59_35.png (42.6 KB) - added by westonruter 7 years ago.
VirtualBox_IE11 - Win7_15_11_2017_08_20_26.png (38.7 KB) - added by westonruter 7 years ago.
42553.poc.diff (2.7 KB) - added by westonruter 7 years ago.
42553.alt.diff (1.5 KB) - added by SergeyBiryukov 7 years ago.
42553.3.diff (2.7 KB) - added by westonruter 7 years ago.
42553.4.diff (3.1 KB) - added by westonruter 7 years ago.
Bump $tinymce_version to bust caches
tinymce.min.js.patch (938.9 KB) - added by azaozz 7 years ago.
tinymce.min.js (469.4 KB) - added by azaozz 7 years ago.
The rebuilt tinymce.min.js
tinymce.min.2.js (469.6 KB) - added by westonruter 7 years ago.
Manually patching minified tinymce.js file

Download all attachments as: .zip

Change History (31)

#1 @SergeyBiryukov
7 years ago

Additional issues with 4.9 in IE11:

  • Cutting content with Ctrl+X doesn't work.
  • Double clicking a word to select it doesn't work.

Both cause the same Object doesn't support property or method 'matches' error.

4.8.3 appears to work as expected.

#2 @SergeyBiryukov
7 years ago

  • Version changed from trunk to 4.9

#3 @afercia
7 years ago

Double clicking a word to select it doesn't work.

Of course, also triple clicking to select a paragraphs doesn't work.

#4 @Clorith
7 years ago

The Object doesn't support property or method 'matches' error also appears when using return to add a newline, needs a double-tap or to be part of a sequence of keystrokes to not be blocked.

I'm also seeing an empty line at the end of the visual editor get moved to the top of the editor when switching back and forth between text and visual mode, although it isn't consistent so I suspect a race condition somewhere for this one.

Can consistently reproduce the losing of paragraphs by quickly swapping between text and visual mode.

This ticket was mentioned in Slack in #core-committers by westonruter. View the logs.


7 years ago

#6 @westonruter
7 years ago

I tried manually fixing the TinyMCE issue via the following (which I don't understand why selectorType is being set to STANDARD in the first place):

  • src/wp-includes/js/tinymce/tinymce.js

    define( 
    1641616416
    1641716417      // As of Chrome 34 / Safari 7.1 / FireFox 34, everyone except IE has the unprefixed function.
    1641816418      // Still check for the others, but do it last.
    16419       else if (selectorType === STANDARD) return elem.matches(selector);
    16420       else if (selectorType === MSSTANDARD) return elem.msMatchesSelector(selector);
     16419      else if (selectorType === STANDARD && elem.matches) return elem.matches(selector);
     16420      else if (selectorType === MSSTANDARD || elem.msMatchesSelector) return elem.msMatchesSelector(selector);
    1642116421      else if (selectorType === WEBKITSTANDARD) return elem.webkitMatchesSelector(selector);
    1642216422      else if (selectorType === FIREFOXSTANDARD) return elem.mozMatchesSelector(selector);
    1642316423      else throw new Error('Browser lacks native selectors'); // unfortunately we can't throw this on startup :(


However, then a different IndexSizeError then occurs.

#7 follow-up: @westonruter
7 years ago

  • Keywords has-patch added; needs-patch removed

What what we could do is just forcibly prevent the wp_keep_scroll_position behavior in IE.

@Clorith please try 42553.poc.diff. It fixes the errors in IE11 for me.

#8 @Clorith
7 years ago

Sorted out the errors on my end as well, also unable to reproduce the loss of paragraphs and other things mentioned in the ticket now.

#9 in reply to: ↑ 7 @SergeyBiryukov
7 years ago

Replying to westonruter:

please try 42553.poc.diff. It fixes the errors in IE11 for me.

Seems to work as expected for me as well.

Given that IE11 is the only supported version though, maybe not enabling wp_keep_scroll_position in the first place would be cleaner? See 42553.alt.diff.

#10 @SergeyBiryukov
7 years ago

There's a similar $is_IE check earlier in the file for editor_expand feature: tags/4.8.3/src/wp-admin/edit-form-advanced.php?marks=33#L23.

@westonruter
7 years ago

#11 @westonruter
7 years ago

  • Keywords needs-testing added
  • Owner set to westonruter
  • Status changed from new to accepted

We found out that the TinyMCE bug is actually because of a clash with MediaElement.js. The is selector lookup function in TinyMCE is using a selectorType function that gets computed at init in the top window where MediaElement.js also loaded. MediaElement.js implements a matches polyfill on Element.prototype, so when selectorType is computed TinyMCE thinks that it is natively available. Later, however, when TinyMCE calls its is function it does so in the context of the TinyMCE iframe window, not the parent window where MediaElement.js has applied its polyfill. In short, TinyMCE really does need to do on the fly feature detection for the matches function. This is implemented in 42553.3.diff. Currently getting the TinyMCE team to review.

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


7 years ago

#13 @westonruter
7 years ago

  • Keywords dev-feedback added; needs-testing removed

I suppose we should bump the $tinymce_version as well?

@westonruter
7 years ago

Bump $tinymce_version to bust caches

#14 @spocke
7 years ago

We will fix this upstream as well the patch looks fine. We where caching the feature detection.

#16 @westonruter
7 years ago

  • Keywords commit added

#17 @SergeyBiryukov
7 years ago

  • Keywords dev-reviewed added; dev-feedback removed

Tested 42553.4.diff, looks good to me.

#18 @westonruter
7 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 42191:

Editor: Disable wp_keep_scroll_position in IE11 since buggy; fix matches polyfill conflict with ME.js by doing runtime feature detection in context window.

Props westonruter, SergeyBiryukov, Clorith for testing.
See #41962, #42029.
Fixes #42553 for trunk.

#19 @westonruter
7 years ago

In 42192:

Editor: Disable wp_keep_scroll_position in IE11 since buggy; fix matches polyfill conflict with ME.js by doing runtime feature detection in context window.

Props westonruter, SergeyBiryukov, Clorith for testing.
See #41962, #42029.
Fixes #42553 for 4.9.

#20 @westonruter
7 years ago

@spocke @afraithe I opened a PR to ephox/sugar with this fix as well: https://github.com/ephox/sugar/pull/1

@azaozz
7 years ago

The rebuilt tinymce.min.js

@westonruter
7 years ago

Manually patching minified tinymce.js file

#21 @westonruter
7 years ago

In 42193:

Editor: Re-build tinymce.min.js after [42191].

See #42553.

#22 @westonruter
7 years ago

In 42194:

Editor: Re-build tinymce.min.js after [42191].

See #42553.

Note: See TracTickets for help on using tickets.