Make WordPress Core

Opened 19 months ago

Last modified 4 weeks ago

#57995 new defect (bug)

Popup flickering in Firefox

Reported by: robehr79's profile robehr79 Owned by:
Milestone: Future Release Priority: normal
Severity: major Version: 6.0
Component: TinyMCE Keywords: has-screenshots has-testing-info needs-patch
Focuses: javascript, administration Cc:

Description (last modified by sabernhardt)

This bug is somehow related to #44911, however, 5 years after that, the bug appears in Firefox. I copy-paste here what I wrote in the forum (https://wordpress.org/support/topic/toolbar-of-tinymce-flickering-in-firefox/):

  1. Begin editing a page in the block editor
  2. Insert a Classic block (TinyMCE)
  3. Mark some text
  4. Turn it into a link
  5. Enter a URL which is wider than the input field, e.g. https://de.wikipedia.org/wiki/Rindfleischetikettierungs%C3%BCberwachungsaufgaben%C3%BCbertragungsgesetz
  6. Close the input field
  7. Reopen the input field
  8. Click on the pencil to edit it
  9. Expected behaviour: Turn it into the input field again. Actual behaviour: Starts flickering and is impossible to work with.

I found the relevant code in the file wp-include/js/tinymce/plugins/wordpress/plugin.js in line 1127 and following, stating:



					/*
					 * Showing a tooltip may trigger a resize event in Chromium browsers.
					 * That results in a flicketing inline menu; tooltips are shown on hovering over a button,
					 * which then hides the toolbar on resize, then it repeats as soon as the toolbar is shown again.
					 */
					if ( event.type === 'resize' || event.type === 'resizewindow' ) {

Funnily enough, there is a remark about this behaviour, but it concerns only Chromium. Firefox uses the event scroll. I tried appending it to the condition, and it really fixed the problem. However, I do not know if just appending || event.type === 'scroll' does not break other things. Actually, I think that the correct solution would be like this: The current condition is about resizing. There should be another condition which caches the previous scrolling position and bails out early if the scroll position did not change altough the event is fired.

This code is present at least since WordPress 6.0 and also in 6.1.1.

Attachments (1)

57995-replication-firefox.gif (814.0 KB) - added by rajinsharwar 15 months ago.

Download all attachments as: .zip

Change History (27)

#1 @sabernhardt
19 months ago

  • Description modified (diff)
  • Focuses javascript administration added
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 6.2.1

Hi and thanks for the report!

It flickered for me, too, in the editor's Classic block but not in the Classic Editor. I used Firefox 111.0.1 in Windows 10, and it still occurs with WordPress 6.2 RC4.

#2 @kelvinballoo
19 months ago

So I want to expand on this more.

I've reproduced the issue with the classic block on a few browsers on PHP 7.4 , WordPress 6.1.1:

  1. Create post
  2. Add 'Classic' block (did not have tinyMCE)
  3. Add text under classic block and assign URL to it.

Here are my observations:
-> Safari (Version 16.3 (17614.4.6.11.6, 17614)) - Works perfect.

-> Chrome (Version 111.0.5563.146 (Official Build) (x86_64)) - There are some flickers that happened when I tried to select the inserted URL (URL is longer than the input field) but it's still workable. The flicker should be fixed though.

-> Firefox (111.0.1 (64 bit))- This is the most problematic.

  • Not only the flicker is much more, you hardly can do anything when you have a long URL. Sometimes the URL popup won't even appear.
  • Short URLs are fine but sometimes the link popup won't disappear after clicking elsewhere on the page.
  • If you have a scenario with 2 links, one which is a short URL which was working fine...and if you insert another long URL somewhere else on the page, this will affect the experience for the short URL too.

#3 @costdev
18 months ago

  • Keywords dev-feedback added
  • Version changed from 6.1.1 to 6.0

Pinging @azaozz as TinyMCE component maintainer to take a look if available. Note the support thread mentioned in this ticket's summary also.

A couple of questions for consideration:

  • Is a fix needed here, or in Gutenberg?
  • This issue has existed in 6.0, 6.1 and now 6.2. Should a fix be committed to trunk only, or should it be backported to the 6.2 and possibly the 6.1 branches?

#4 @parrishh
18 months ago

#58072 was marked as a duplicate.

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


17 months ago

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


17 months ago

#7 @audrasjb
17 months ago

  • Milestone changed from 6.2.1 to 6.2.2

@costdev it has to be committed to trunk and backported to branch 6.2 only :)

As per today's bug scrub, let's move this ticket to 6.2.2.
If a patch is added to the ticket before tomorrow 15:00 UTC and WP 6.2.1 RC1, feel free to move it back to milestone 6.2.1!

#8 @robehr79
17 months ago

Sorry for coming up this way by simply putting the patch verbatim here, but I think that this is the fix to the flickering, as patch generated with Git:

--- a/wp-includes/js/tinymce/plugins/wordpress/plugin.js
+++ b/wp-includes/js/tinymce/plugins/wordpress/plugin.js
@@ -790,12 +790,14 @@ tinymce.PluginManager.add( 'wordpress', function( editor ) {
                        currentSelection,
                        timeout,
                        container = editor.getContainer(),
+                       editorContainer = document.querySelector(".interface-navigable-region.interface-interface-skeleton__content"),
                        wpAdminbar = document.getElementById( 'wpadminbar' ),
                        mceIframe = document.getElementById( editor.id + '_ifr' ),
                        mceToolbar,
                        mceStatusbar,
                        wpStatusbar,
-                       cachedWinSize;
+                       cachedWinSize,
+                       cachedScrollPos;
 
                        if ( container ) {
                                mceToolbar = tinymce.$( '.mce-toolbar-grp', container )[0];
@@ -1154,6 +1156,19 @@ tinymce.PluginManager.add( 'wordpress', function( editor ) {
                                                }
                                        }
 
+                                       if ( event.type === "scroll" || event.type === "scrollwindow" ) {
+
+                                               if ( cachedScrollPos ) {
+                                                       if ( editorContainer.scrollLeft === cachedScrollPos.X && editorContainer.scrollTop === cachedScrollPos.Y ) return;
+                                               } else {
+                                                       cachedScrollPos = {
+                                                               X: editorContainer.scrollLeft,
+                                                               Y: editorContainer.scrollTop,
+                                                       };
+                                                       return;
+                                               }
+                                       }
+
                                        clearTimeout( timeout );
 
                                        timeout = setTimeout( function() {
@@ -1165,6 +1180,7 @@ tinymce.PluginManager.add( 'wordpress', function( editor ) {
 
                                        activeToolbar.scrolling = true;
                                        activeToolbar.hide();
+                                       cachedScrollPos = null;
                                }
                        }
                }
-- 

Feel free to improve it accordingly. Sadly enough, this fixes the flickering but there is another issue: The toolbar does not close reliably when it loses the focus, neither in Firefox nor in Chrome, neither with nor without this fix.

Sorry, I won't have time to investigate deeper. I hope my contribution helps.

Kind regards,
Robert

Last edited 17 months ago by robehr79 (previous) (diff)

#9 @desrosj
17 months ago

  • Milestone changed from 6.2.2 to 6.2.3

Moving this to 6.2.3.

#10 @lgladdy
17 months ago

This also happens in Chrome when TinyMCE is being displayed in a meta box with a position set to side in the classic editor.

Last edited 17 months ago by lgladdy (previous) (diff)

#11 @mrinal013
16 months ago

  • Keywords needs-testing added

In WordPress Core, the version of TinyMCE is 4.9.11
The latest version of TinyMCE is 5.10.7 - December 6, 2022
Is this issue happen for updating?

#12 @rajinsharwar
15 months ago

  • Keywords has-screenshots added; needs-testing removed

Hey all, the issue does seem to be still replicable in 6.2.2 but is solved in 6.3 RC@ because of the new popup system while using classic blocks.

#13 @SergeyBiryukov
14 months ago

  • Milestone changed from 6.2.3 to 6.4

Moving to 6.4 for now, as there are no plans for 6.2.3 at this time, and this does not appear to be a regression in 6.3.

If this is indeed fixed in 6.3 as noted in comment:12, would be great to track down the exact commit.

#14 @robertehrenleitnerplus
14 months ago

It's still not fixed. Neither do I see my suggested fix, nor in any other way.

#15 @oglekler
14 months ago

comment:8 has a proposed fix in plain text. We need to make a patch file from it, and it will be clear if it will solve the issue or something else should be done.

#16 @robertehrenleitnerplus
13 months ago

Whoops, I just noticed that I have two accounts. @robehr79 and @robertehrenleitnerplus is both me.

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


12 months ago

This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.


12 months ago

#19 @nicolefurlan
12 months ago

  • Keywords needs-testing has-testing-info added; dev-feedback removed

This ticket was discussed during the 6.4 bug scrub today. It looks like the best path forward would be to determine whether the issue is fixed in 6.3 per #comment:12, and if it is, determine the exact commit where it was fixed per #comment:13.

#20 @oglekler
12 months ago

  • Keywords needs-testing removed
  • Milestone changed from 6.4 to 6.5

I also can confirm, that the problem is still present, but the patch cannot be applied even for testing purposes, because we have quite a different code in the target file already. So, we need a new patch.

https://github.com/WordPress/wordpress-develop/blob/trunk/src/js/_enqueues/vendor/tinymce/plugins/wordpress/plugin.js - this is the file we need to make changes into and then make a build. As far as I understand, this is our own file and not a vendor's one.

I don't see that it is possible to make in quick and get into the current milestone, so, I am moving this ticket forward.

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


8 months ago

#22 @audrasjb
8 months ago

  • Milestone changed from 6.5 to Future Release

As per today's bug scrub, we decided to move this ticket to Future Release to give it more time to get a patch.

#23 @robertehrenleitnerplus
8 months ago

The patch I provided 9 months ago can still be applied to the current version 6.4.3, as I have just made it on my own. Additionally, I found out that it is possible to use container instead of the newly introduced containerElement. Not even the line numbers have changed much:

--- a/wp-includes/js/tinymce/plugins/wordpress/plugin.js
+++ b/wp-includes/js/tinymce/plugins/wordpress/plugin.js
@@ -795,7 +795,8 @@ tinymce.PluginManager.add( 'wordpress', function( editor ) {
                        mceToolbar,
                        mceStatusbar,
                        wpStatusbar,
-                       cachedWinSize;
+                       cachedWinSize,
+                       cachedScrollPos;
 
                        if ( container ) {
                                mceToolbar = tinymce.$( '.mce-toolbar-grp', container )[0];
@@ -1154,6 +1155,20 @@ tinymce.PluginManager.add( 'wordpress', function( editor ) {
                                                }
                                        }
 
+                                       if ( event.type === 'scroll' || event.type === 'scrollwindow' ) {
+                                               if ( cachedScrollPos ) {
+                                                       if ( container.scrollLeft === cachedScrollPos.X && container.scrollTop === cachedScrollPos.Y ) {
+                                                               return;
+                                                       }
+                                               } else {
+                                                       cachedScrollPos = {
+                                                               X: container.scrollLeft,
+                                                               Y: container.scrollTop
+                                                       };
+                                                       return;
+                                               }
+                                       }
+
                                        clearTimeout( timeout );
 
                                        timeout = setTimeout( function() {
@@ -1165,6 +1180,7 @@ tinymce.PluginManager.add( 'wordpress', function( editor ) {
 
                                        activeToolbar.scrolling = true;
                                        activeToolbar.hide();
+                                       cachedScrollPos = null;
                                }
                        }
                }

#24 @azaozz
5 weeks ago

Does this still happen? I'm not able to reproduce it in Firefox and Chromium in trunk, but this has always been very hard to consistently reproduce.

Also, seems this may be CSS related. Would be great if someone that has the time could troubleshoot the CSS and find out if a particular style may be causing it.

#25 @sabernhardt
5 weeks ago

The flickering should be rarer now; I do not experience it with the new Classic block modal. However, Firefox can still have the problem when the block editor is not in an iframe (showing the Custom Fields panel or activating a plugin such as Yoast SEO would keep the non-framed editor).

The toolbar element (I found #mceu_18 near the end of the body) repeatedly adds and removes display: none to the style attribute. The script comment from #44911 mentions flickering (or "flicketing") in Chromium. I was able to change the flashing rate with a different timeout value, which indicates that the resize section runs in Firefox, too.

Last edited 5 weeks ago by sabernhardt (previous) (diff)

#26 @robertehrenleitnerplus
4 weeks ago

Can you provide me a URL or a ZIP file with the fix? I would try it immediately, as it is part of our continuous quality assurance.

Note: See TracTickets for help on using tickets.