Contrib.social

Reduce setTimeout calls as they can cause potential race conditions: e.g. CKEditor 4

Open on Drupal.org →
Created on 16 January 2024, 4 months ago
Updated 6 February 2024, 3 months ago

Problem/Motivation

Following the update to Gutenberg 3.0.x, we've change certain behaviour to run behind a 200ms timeout, leading to potential race conditions and a change in existing behaviour.

We should ideally remove all timeout related calls in favour of something a bit more consistent.

This change breaks integration with CKEditor 4.

Steps to reproduce

Install or make use of CKEditor 4 (JS version 4.18.0 specifically in my test case).

Have a text field that makes use of CKEditor in the additional Node field.

Refresh the page continuously until some javascript errors are triggered due to the use of timeout to handle the editor initializations/processing - in this particular case, CKEditor is initialized and has a MutationObserver which starts listening to other Gutenberg related DOM changes that aren't related to it.

The stack trace is something along the lines of:

ckeditor.js?v=4.18.0:72 Uncaught TypeError: Cannot read properties of undefined (reading 'ownerDocument')
    at $.getDocument (ckeditor.js?v=4.18.0:72:186)
    at $.getData (ckeditor.js?v=4.18.0:1226:332)
    at $.<anonymous> (ckeditor.js?v=4.18.0:375:6)
    at b.d (ckeditor.js?v=4.18.0:10:246)
    at b.<anonymous> (ckeditor.js?v=4.18.0:12:91)
    at window.CKEDITOR.window.CKEDITOR.dom.CKEDITOR.editor.CKEDITOR.editor.fire (ckeditor.js?v=4.18.0:13:285)
    at b.getData (ckeditor.js?v=4.18.0:274:414)
    at d (ckeditor.js?v=4.18.0:1218:85)
    at MutationObserver.<anonymous> (ckeditor.js?v=4.18.0:1217:501)

ckeditor.js?v=4.18.0:439 Uncaught TypeError: Cannot read properties of null (reading 'isInline')
    at b.<anonymous> (ckeditor.js?v=4.18.0:439:378)
    at b.d (ckeditor.js?v=4.18.0:10:246)
    at b.<anonymous> (ckeditor.js?v=4.18.0:12:91)
    at window.CKEDITOR.window.CKEDITOR.dom.CKEDITOR.editor.CKEDITOR.editor.fire (ckeditor.js?v=4.18.0:13:285)
    at $.<anonymous> (ckeditor.js?v=4.18.0:1214:3)
    at ckeditor.js?v=4.18.0:28:254

Ideally, this is something that'd be fixed within the CKEditor library, but the change in behaviour still stands, and I believe we should handle these changes (at least the metabox movements) as soon as possible unless there's a specific reason to wait.

Proposed resolution

Remove/reduce all usages of setTimeouts within the codebase unless necessary for a specific use-case.

Remaining tasks

Provide issue fork/patch.

User interface changes

N/A

API changes

N/A

📌 Task
Status

Fixed

Version

3.0

Component

Code

Created by

codebymikey

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Pipeline finished with Skipped
    11 months ago
    #12401
  • Pipeline finished with Skipped
    4 months ago
    #73122
  • Issue created by @codebymikey
  • Merge request !92Issue #3414978: Reduce setTimeout calls as they can cause potential race conditions → (Merged) created by codebymikey
  • Pipeline finished with Success
    4 months ago
    #78296
  • Status changed to Needs review 4 months ago
  • Comment 4 months ago →
    codebymikey

    Pushed a fix utilizing the MutationObserver API to attach the metaboxes as soon as the metabox container is available in the DOM (which I believe was the sole reason for the original call)

    Also addressed the Drupal fullscreen button logic to reduce the number of setTimeouts and CSS queries.

  • Pipeline finished with Success
    4 months ago
    #78625
  • First commit to issue fork.
  • Pipeline finished with Failed
    4 months ago
    #78657
  • Pipeline finished with Success
    4 months ago
    #79963
  • Pipeline finished with Success
    4 months ago
    #79966
  • Comment 4 months ago →
    System Message
  • Status changed to Fixed 4 months ago
  • Comment 4 months ago →
    🇵🇹Portugal marcofernandes
  • Pipeline finished with Skipped
    4 months ago
    #80053
  • Merge request !97Issue #3414978: Handle brief page title flicker on Firefox → (Merged) created by codebymikey
  • Pipeline finished with Success
    4 months ago
    #80675
  • Comment 4 months ago →
    codebymikey

    Addressed the Firefox page title flicker issue discussed in private chat.

    The flicker doesn't happen in Chrome, but before the original issue was addressed in fd00464, a potential race condition in Gutenberg can cause <PostTypeSupportCheck supportKeys="title"> to render titles even though it's not actively supported by the post type.

    We should only flag the editor as ready once we can successfully resolve the post type to avoid any other knock-off race condition issues.

  • Pipeline finished with Success
    4 months ago
    #80679
  • Status changed to Needs review 4 months ago
  • Comment 4 months ago →
    System Message
  • Pipeline finished with Skipped
    4 months ago
    #81451
  • Status changed to Fixed 4 months ago
  • Comment 4 months ago →
    🇵🇹Portugal marcofernandes
  • Pipeline finished with Success
    4 months ago
    #83261
  • Pipeline finished with Skipped
    4 months ago
    #83360
  • Pipeline finished with Success
    4 months ago
    Total: 159s
    #84070
  • Pipeline finished with Failed
    4 months ago
    Total: 150s
    #84078
  • Pipeline finished with Failed
    4 months ago
    Total: 150s
    #84088
  • Pipeline finished with Success
    4 months ago
    Total: 180s
    #84139
  • Pipeline finished with Success
    4 months ago
    #84149
  • Pipeline finished with Success
    4 months ago
    Total: 162s
    #84155
  • Pipeline finished with Success
    4 months ago
    Total: 598s
    #85672
  • Pipeline finished with Skipped
    4 months ago
    #86464
  • Comment 3 months ago →
    System Message

    Automatically closed - issue fixed for 2 weeks with no activity.

  • Pipeline finished with Skipped
    3 months ago
    #90185
  • Pipeline finished with Success
    3 months ago
    #92414
  • Pipeline finished with Canceled
    3 months ago
    Total: 792s
    #92424
  • Pipeline finished with Failed
    3 months ago
    Total: 871s
    #92428
  • Pipeline finished with Success
    3 months ago
    Total: 955s
    #92928
  • Pipeline finished with Success
    3 months ago
    Total: 932s
    #93067
  • Pipeline finished with Success
    3 months ago
    Total: 251s
    #93760
  • Pipeline finished with Success
    3 months ago
    Total: 264s
    #93771
  • Pipeline finished with Skipped
    3 months ago
    #95191
  • Pipeline finished with Skipped
    3 months ago
    #97552
  • Pipeline finished with Success
    about 2 months ago
    #125213
  • Pipeline finished with Success
    about 2 months ago
    Total: 183s
    #125223
  • Pipeline finished with Failed
    about 2 months ago
    Total: 218s
    #125679
  • Pipeline finished with Success
    about 2 months ago
    #125690
  • Pipeline finished with Success
    about 2 months ago
    #126538
  • Pipeline finished with Success
    about 2 months ago
    #126542
  • Pipeline finished with Skipped
    about 2 months ago
    #128511
  • Pipeline finished with Success
    about 2 months ago
    #132753
  • Pipeline finished with Success
    26 days ago
    Total: 858s
    #152984
  • Pipeline finished with Skipped
    25 days ago
    #153998
  • Pipeline finished with Skipped
    14 days ago
    #164341
  • Pipeline finished with Skipped
    9 days ago
    #168631
contrib.social Blog FAQ Discussions
Production build 0.67.2 2024