Skip to content
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

Chrome version 58 causes problems with selections in the tinymce editor #9518

Closed
hostep opened this issue May 4, 2017 · 11 comments
Closed
Labels
Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development

Comments

@hostep
Copy link
Contributor

hostep commented May 4, 2017

Preconditions

  1. Magento CE 2.1.6 or develop branch, doesn't matter
  2. Google Chrome browser, version 58.0.3029.81 or 58.0.3029.96 or 58.0.3029.110

Steps to reproduce

  1. Start editing a CMS page in the backend of Magento
  2. Make sure you have a CMS page which contains an image and/or a widget
  3. Try to select the image or the widget, by clicking it.

Expected result

  1. Image or widget is selected

Actual result

  1. Nothing is selected, and we get an error in the console:
tiny_mce_src.js:1173 Uncaught DOMException: Failed to execute 'setBaseAndExtent' on 'Selection': There is no child at offset 1.
    at Editor.<anonymous> (https://magento2-test.whatever/static/version1493836737/adminhtml/Magento/backend/en_US/tiny_mce/tiny_mce_src.js:1173:27)
    at Dispatcher.dispatch (https://magento2-test.whatever/static/version1493836737/adminhtml/Magento/backend/en_US/tiny_mce/tiny_mce_src.js:538:13)
    at DOMUtils.eventHandler (https://magento2-test.whatever/static/version1493836737/adminhtml/Magento/backend/en_US/tiny_mce/tiny_mce_src.js:12740:34)
    at HTMLDocument.cb (https://magento2-test.whatever/static/version1493836737/adminhtml/Magento/backend/en_US/tiny_mce/tiny_mce_src.js:7066:14)

Discussion

This doesn't happen in Safari 10.1 or Firefox 53.0. I'm pretty sure this used to work in Chrome 57 but haven't verified this.
The commit log for Chrome between version 57.0.2987.133 and 58.0.3029.81 contains a bunch of updates/fixes to how they implemented the Selection API
More specific: https://www.chromestatus.com/feature/5696359768260608

From the source code of TinyMCE, it looks like they used the setBaseAndExtent method as a workaround for an old bug in the WebKit rendering engine.

If I remove this workaround and use a normal select call, everything seems to be working again in Chrome 58, and also keeps working in Safari 10.1
So this might be the solution then:

diff --git a/lib/web/tiny_mce/tiny_mce_src.js b/lib/web/tiny_mce/tiny_mce_src.js
index 06f953b379a..7189ca9e159 100644
--- a/lib/web/tiny_mce/tiny_mce_src.js
+++ b/lib/web/tiny_mce/tiny_mce_src.js
@@ -1166,11 +1166,8 @@ tinymce.create('static tinymce.util.XHR', {
                ed.onClick.add(function(ed, e) {
                        e = e.target;

-                       // Workaround for bug, http://bugs.webkit.org/show_bug.cgi?id=12250
-                       // WebKit can't even do simple things like selecting an image
-                       // Needs tobe the setBaseAndExtend or it will fail to select floated images
                        if (/^(IMG|HR)$/.test(e.nodeName))
-                               ed.selection.getSel().setBaseAndExtent(e, 0, e, 1);
+                               ed.selection.select(e);

                        if (e.nodeName == 'A' && ed.dom.hasClass(e, 'mceItemAnchor'))
                                ed.selection.select(e);

This is also how they fixed it in TinyMCE 4.5.4 a month or two ago: tinymce/tinymce@19f3098#diff-e5490c44bb1973bd0210940a7c159866

There is a bug reported on the Chromium bugtracker, because Wordpress has the same problem, but the bugreport has status WontFix, so I think Chrome is going to continue to have this issue unless we fix it in Magento.
Wordpress thread: https://core.trac.wordpress.org/ticket/40305
Wordpress fixed it by simple upgrading TinyMCE to the latest version, but I'm affraid we can't do that in Magento, since it is using TinyMCE version 3.4.7, which was released in 2011, and I think upgrading will most likely break everything (untested).

@korostii
Copy link
Contributor

korostii commented May 4, 2017

Related: #8874, #7149 and #700

@TommyKolkman
Copy link

I can second this bug, Chrome 58. Confirmed that it didn't happen on 57 (colleague hadn't updated yet).

@TommyKolkman
Copy link

Also: a client of mine reports this is coming up on Microsoft Edge.

@hostep
Copy link
Contributor Author

hostep commented May 5, 2017

If some Magento dev, or maybe even @spocke can confirm that my suggested fix above is correct, then I'll create a PR against both the develop and 2.1-develop branch so we can try to include this fix in one of the next Magento versions.

@TommyKolkman
Copy link

I've implemented and tested @hostep's fix - all works well.

@KrystynaKabannyk
Copy link

Hi @hostep, Thank you for the contribution, I'm closing this issue.

@korostii
Copy link
Contributor

korostii commented Jun 1, 2017

@KrystynaKabannyk, why is this being closed? Has the fix reached a 2.1.x release already?

Until the fix gets into a released version, this issue will persist for ordinary merchants.

BTW, Are there any plans to fix it on other branches as per hostep's comment?

@KrystynaKabannyk
Copy link

Sorry for closing in a hurry, yes , you are right, we need to wait until patch release.

@magento-team
Copy link
Contributor

Internal ticket to track issue progress: MAGETWO-69234

@magento-team magento-team added 2.1.x Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development labels Jul 31, 2017
@magento-team
Copy link
Contributor

Internal ticket to track issue progress: MAGETWO-69152

@hostep
Copy link
Contributor Author

hostep commented Sep 9, 2017

Let's close this issue, since it was fixed in Magento 2.1.8

@hostep hostep closed this as completed Sep 9, 2017
nikolas added a commit to ccnmtl/mediathread that referenced this issue Nov 4, 2019
This may fix sentry error MEDIATHREAD-5G.

See: magento/magento2#9518
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development
Projects
None yet
Development

No branches or pull requests

6 participants