magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.56k stars 9.32k forks source link

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

Closed hostep closed 7 years ago

hostep commented 7 years ago

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: https://github.com/tinymce/tinymce/commit/19f3098f2bbd4740b4e9ac67b40c2a58201fc386#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 commented 7 years ago

Related: #8874, #7149 and #700

TommyKolkman commented 7 years ago

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

TommyKolkman commented 7 years ago

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

hostep commented 7 years ago

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 commented 7 years ago

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

KrystynaKabannyk commented 7 years ago

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

korostii commented 7 years ago

@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 commented 7 years ago

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

magento-team commented 7 years ago

Internal ticket to track issue progress: MAGETWO-69234

magento-team commented 7 years ago

Internal ticket to track issue progress: MAGETWO-69152

hostep commented 7 years ago

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