2002481 - Latest nightly versions seem to have broken CKEditor 4

Status: VERIFIED FIXED

(Core :: DOM: Navigation, defect)

Description

[Adrian London]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:147.0) Gecko/20100101 Firefox/147.0

Steps to reproduce:

A form I frequent appears to use CKEditor 4 for their posting editor window. For around 2 days, this has not worked.

A Nightly from 22nd November works. I've not yet gone through every version of Nightly since then to check the exact break-point.

In the current Nightly, this fails:
https://ckeditor.com/ckeditor-4/demo/

It works in other browsers and older Nightly versions.

(
The newer CKEditor 5 does well, but not every website uses it:
https://ckeditor.com/ckeditor-5/demo/feature-rich/
)

Actual results:

A blank textbox appears which can't be typed into. None of the font/format controls are clickable.

Expected results:

It should allow me to type as it did previously.

Comment 1

[Adrian London]

The forum is https://www.flyertalk.com/forum/ although it's quicker to test using the ckeditor demo link above.

Comment 2

[Alice0775 White]

Regression window:
https://hg-edge.mozilla.org/integration/autoland/pushloghtml?fromchange=37f1c536fdb89cd69039774a87f3ad8a94b95328&tochange=86c4f03a43058ea62f76021ad5a50e6199c34567

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 543435

:vhilla, since you are the author of the regressor, bug 543435, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 4

[Vincent Hilla [:vhilla]]

If I spoof my user agent to correspond to Chrome 142.0.0.0, then the editor works for me.

Comment 5

[Henri Sivonen (:hsivonen)]

vhilla says on Matrix that UA spoofing fixes this.

Comment 6

[Henri Sivonen (:hsivonen)]

It checks CKEDITOR.env.gecko in various places. CKEDITOR.env.chrome and CKEDITOR.env.webkit exist.

Perhaps we could build a cross-site intervention to modify CKEDITOR.env?

Also, for customers, there is:

optional extra
Third-party API Changes Assurance

CKSource resolves* any critical / high severity issues in CKEditor 4 caused by changes in a third-party apps (e.g. browser, MS Word) that CKEditor 4 depends on for its functionality.

So perhaps we should let CKSource know so that they can fix this upstream and distribute the fix per above.

Comment 7

[Neoony]

Same issue / duplicate of https://bugzilla.mozilla.org/show_bug.cgi?id=2002451
Although this one is more specific about the CKEditor and now has more info
Just to mention/link
I will resolve 2002451 as duplicate, although its older

Comment 9

[Vincent Hilla [:vhilla]]

https://github.com/ckeditor/ckeditor4/blob/c7e59ec199298b6b23f4aa7a7668f18572385bac/plugins/wysiwygarea/plugin.js#L48-L50

// Asynchronous iframe loading is only required in IE>8 and Gecko (other reasons probably).
// Do not use it on WebKit as it'll break the browser-back navigation.
var useOnloadEvent = ( CKEDITOR.env.ie && !CKEDITOR.env.edge ) || CKEDITOR.env.gecko;

Comment 10

[Henri Sivonen (:hsivonen)]

To expand on comment 9 for folks following along here (as opposed to Matrix):

The likely cause is the code seen at
https://github.com/ckeditor/ckeditor4/blob/c7e59ec199298b6b23f4aa7a7668f18572385bac/plugins/wysiwygarea/plugin.js#L48-L50
and
https://github.com/ckeditor/ckeditor4/blob/c7e59ec199298b6b23f4aa7a7668f18572385bac/core/dom/element.js#L72

If the browser has been sniffed as Gecko, CKEditor 4 assumes that it can create an iframe element with empty src by parsing a source snippet in an element connected to the DOM, then detach the iframe, then set onload, then re-insert it.

Now onload has already fired before CKEditor 4 sets the onload handler.

Creating the iframe element using document.createElement and setting the onload handler before the iframe has ever been inserted to the document would work both with Gecko's old about:blank and with the new Chrome-compatible about:blank.

Comment 11

[Vincent Hilla [:vhilla]]

(In reply to Henri Sivonen (:hsivonen) from comment #6)

Perhaps we could build a cross-site intervention to modify CKEDITOR.env?

In case we need a fix for editors that aren't updated anymore, another option is to dispatch load on all .cke_wysiwyg_frame elements. I'm unsure if that is feasible as intervention. Duplicate load events are fine as they remove the listener. Modifying env might break other parts.

Comment 12

[Henri Sivonen (:hsivonen)]

(In reply to Vincent Hilla [:vhilla] from comment #11)

In case we need a fix for editors that aren't updated anymore, another option is to dispatch load on all .cke_wysiwyg_frame elements. I'm unsure if that is feasible as intervention. Duplicate load events are fine as they remove the listener. Modifying env might break other parts.

Probably feasible as a Gecko hard-coded intervention but not as intervention using the usual intervention machinery.

Comment 13

[Vincent Hilla [:vhilla]]

(In reply to Henri Sivonen (:hsivonen) from comment #10)

Creating the iframe element using document.createElement and setting the onload handler before the iframe has ever been inserted to the document would work both with Gecko's old about:blank and with the new Chrome-compatible about:blank.

I think CKEDITOR.dom.element.createFromHtml is fine, because re-inserting the iframe will cause another sync load event:

ifr = document.createElement("iframe")
ifr.onload = console.log
document.body.append(ifr)
ifr.remove()
document.body.append(ifr)
console.log("done")

Comment 14

[Vincent Hilla [:vhilla]]

(In reply to Henri Sivonen (:hsivonen) from comment #6)

Also, for customers, there is:

optional extra
Third-party API Changes Assurance

CKSource resolves* any critical / high severity issues in CKEditor 4 caused by changes in a third-party apps (e.g. browser, MS Word) that CKEditor 4 depends on for its functionality.

So perhaps we should let CKSource know so that they can fix this upstream and distribute the fix per above.

I reached out to their support.

Comment 15

[Henri Sivonen (:hsivonen)]

Making a note in case we want a reasonable-performance way of identifying CKEditor 4 below some presumed future version that might fix this:

We might be able to rely on the script URL ending with "/ckeditor.js" plus possibly a query string.

Failing that or as an additional signal we could check if the script source starts with (possibly allowing for earlier years in place of "2025"):

/*
Copyright (c) 2003-2025, CKSource Holding sp. z o.o. All rights reserved.

(Notably, the next line seems to vary, so we shouldn't be looking at the next line.)

If that kind of fail-fast test makes a script look like CKEditor 4, there should be a substring of the form:

{timestamp:"PAO8",version:"4.25.1-lts (Standard)",revision:"c7e59ec199",rnd:Math

with the quoted strings varying relatively close to the start of the script.

Comment 16

[Henri Sivonen (:hsivonen)]

Neoony, would the heuristic described in the previous comment have worked for the self-hosted system that you experienced this problem with?

Comment 17

[Henri Sivonen (:hsivonen)]

The minified condition CKEDITOR.env.ie&&!CKEDITOR.env.edge||CKEDITOR.env.gecko seems to occur only once.

Comment 18

[Neoony]

(In reply to Henri Sivonen (:hsivonen) from comment #16)

Neoony, would the heuristic described in the previous comment have worked for the self-hosted system that you experienced this problem with?

Yes, I used "User-Agent Switcher and Manager" and switched to Chrome 142 and now I can type into the text box and it just works fine in topdesk now. No other issues noticed.

Comment 19

[Adrian London]

If it's any help, inspecting an edit box in https://www.flyertalk.com/forum/ shows the following for ckeditor.js

Copyright (c) 2003-2025, CKSource Holding sp. z o.o. All rights reserved.
For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license/

There's also this, in config.js (I'm not a developer so no idea if this is useful or not):

Copyright (c) 2003-2025, CKSource Holding sp. z o.o. All rights reserved.
For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license/

User-agent spoofing is my current workaround.

Comment 20

[Adrian London]

Hmm - can't edit my post. Second code snipped is:

 Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved.
 For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license

Comment 21

[BugBot [:suhaib / :marco/ :calixte]]

The bug is marked as tracked for firefox147 (nightly). We have limited time to fix this, the soft freeze is in 7 days. However, the bug still isn't assigned.

:hsinyi, could you please find an assignee for this tracked bug? Given that it is a regression and we know the cause, we could also simply backout the regressor. If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Comment 23

[Henri Sivonen (:hsivonen)]

Taking for the purposes of workaround for old CKEditor 4 instances that haven't been updated.

Comment 24

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(Thank you, Henri)

Comment 25

[Henri Sivonen (:hsivonen)]

Attachment: Bug 2002481 - Patch UA sniffing in CKEditor 4 for about:blank compatibility.

Comment 26

[Henri Sivonen (:hsivonen)]

Shippable-config builds coming up in:
https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=166023

Comment 27

[Henri Sivonen (:hsivonen)]

https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=166094

The domain exclusion list doesn't appear to actually work. Still requesting review in order to unbreak Nightly sooner than later.

Comment 28

[Emilio Cobos Álvarez [:emilio]]

So... I think the code looks ok / better than nothing. But I suspect we can do better?

In particular, this patch assumes minified CKEditor in a standalone file, and I suspect it's going to be fairly frequent to just have ckeditor bundled with the rest of your JS dependencies.

The relevant code is:

• https://github.com/ckeditor/ckeditor4/blob/c7e59ec199298b6b23f4aa7a7668f18572385bac/plugins/wysiwygarea/plugin.js#L43

I think we can exploit some facts about that code:

• The iframe always has src="".
• Always has the cke_wysiwyg_frame and cke_reset classes.

Interestingly, that class is not used in ckeditor 5:

• https://github.com/search?q=repo%3Ackeditor%2Fckeditor5%20cke_wysiwyg_frame&type=code

So an alternative, maybe more robust approach, could be to do something like this on HTMLIFrameElement::BindToTree():

if (MayHaveClass() && HasAttr(nsGkAtoms::src) && !HasNonEmptyAttr(nsGkAtoms::src)) {
  if (auto* classes = GetClasses()) {
    if (classes->Contains(nsGkAtoms::cke_wysiwyg_frame)) {
      new AsyncEventDispatcher(this, u"load"_ns, ...)->PostDomEvent();
    }
  }
}

Or so, with the relevant use counter + skip list + function warning.

Has such an approach been considered?

Comment 30

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

if (MayHaveClass() && HasAttr(nsGkAtoms::src) && !HasNonEmptyAttr(nsGkAtoms::src)) {
  if (auto* classes = GetClasses()) {
    if (classes->Contains(nsGkAtoms::cke_wysiwyg_frame)) {
      new AsyncEventDispatcher(this, u"load"_ns, ...)->PostDomEvent();
    }
  }
}

If we could check some global objects which is specific thing of CKEditor, this hack could work with customized instances which do not use cke_wysiwyg_frame class. I did such hack at changing the keypress event behavior, see KeyPressEventModelCheckerChild. Although I'm not sure whether this is reasonable for the load event dispatching.

Comment 31

[Henri Sivonen (:hsivonen)]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #28)

In particular, this patch assumes minified CKEditor in a standalone file, and I suspect it's going to be fairly frequent to just have ckeditor bundled with the rest of your JS dependencies.

Yes, the assumption that the minified CKEditor looks like it came from CKEditor's own minification workflow is the main weakness of the appoach in the patch. However, 1) the cases seen so far indeed have come from the kind of minification workflow that makes the patch work and 2) CKEditor 4 seems to be designed to be used like this: It seems that the intent is that a site using CKEditor 4 first evaluates a short configuration script and then evaluates CKEditor 4 as a separate script (that looks like what the patcher code that I wrote is checking for).

The relevant code is:

• https://github.com/ckeditor/ckeditor4/blob/c7e59ec199298b6b23f4aa7a7668f18572385bac/plugins/wysiwygarea/plugin.js#L43

I think we can exploit some facts about that code:

• The iframe always has src="".
• Always has the cke_wysiwyg_frame and cke_reset classes.

Interestingly, that class is not used in ckeditor 5:

• https://github.com/search?q=repo%3Ackeditor%2Fckeditor5%20cke_wysiwyg_frame&type=code

So an alternative, maybe more robust approach, could be to do something like this on HTMLIFrameElement::BindToTree():

if (MayHaveClass() && HasAttr(nsGkAtoms::src) && !HasNonEmptyAttr(nsGkAtoms::src)) {
  if (auto* classes = GetClasses()) {
    if (classes->Contains(nsGkAtoms::cke_wysiwyg_frame)) {
      new AsyncEventDispatcher(this, u"load"_ns, ...)->PostDomEvent();
    }
  }
}

Or so, with the relevant use counter + skip list + function warning.

Has such an approach been considered?

Yes, this was considered.

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #30)

If we could check some global objects which is specific thing of CKEditor, this hack could work with customized instances which do not use cke_wysiwyg_frame class.

I interpreted the discussion on the Web Compatibility channel as discouragement to go look at the global object, though on a second read it wasn't actually discouragement against looking at the global though it was discouragement against applying the special case to possible future versions of CKEditor 4.

The reasons why I wrote the patch the way I did as opposed to looking for the class on an iframe being bound to tree and looking at the global are:

• It seemed bad to create a special behavior that affects future scripts as opposed to targeting only versions of CKEditor 4 in existence today, and it was simpler to do the version check by running a regexp on the script source (with a high-performance early exit if the script doesn't look CKEditor-like enough to try run the regexp) compared to walking the JavaScript object tree from C++ to identify CKEditor version.
• Since we now have a Chrome-compatible about:blank and CKEditor 4 already has code that's compatible with Chrome-compatible about:blank behind a unique expression, it seemed better to make use of the code path that already exists in CKEditor 4 compared to making Gecko do something that creates a new weird behavior (an iframe firing an extra event if it has a particular class).

I suppose what seems the best way depends a lot on what failure mode we want to avoid more: Accidentally applying an intervention too narrowly or accidentally applying an intervention too broadly. I was optimizing for avoiding making the intervention apply too broadly.

Comment 32

[Emilio Cobos Álvarez [:emilio]]

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #30)

If could check some global objects which is specific thing of CKEditor, this hack could work with customized instances which do not use cke_wysiwyg_frame class.

Can you elaborate? When would that happen, if an app forks ckeditor or something? I don't think we should worry about that in practice. Not applying the intervention in that case seems still better than rewriting the script to me?

Comment 33

[Henri Sivonen (:hsivonen)]

https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=166307

Comment 34

[Emilio Cobos Álvarez [:emilio]]

Attachment: Test-case of what ckeditor is doing

It seems we're not firing the load event at all. This logs in Chrome but not in Nightly. Seems that's the root cause of the issue and fixing that probably solves this.

Comment 35

[Henri Sivonen (:hsivonen)]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #34)

It seems we're not firing the load event at all. This logs in Chrome but not in Nightly. Seems that's the root cause of the issue and fixing that probably solves this.

On Matrix, the description of Nightly behavior here was affected by an add-on. (Concerning on its own, but not the issue here.)

Comment 36

[Emilio Cobos Álvarez [:emilio]]

So apparently comment 34 does fire the load event on a clean profile, but not on my regular profile. Removing the SingleFile extension makes it work on file:// but not on https://bug2002481.bmoattachments.org/attachment.cgi?id=9530020.

So there's probably some extension mechanisms that are interacting poorly with the sync about:blank...

Comment 37

[Henri Sivonen (:hsivonen)]

Attachment: WIP: Bug 2002481 - Fire an extra load event for CKEditor 4 wysiwyg editor iframes.

Comment 38

[Adrian London]

Speaking of extensions ... around the same time I hit this issue, I also noticed that Bitwarden (and other extensions, but I mainly use that one with shortcut keys) ignore shortcut keys.

So, ⌘⇧L should open the Bitwarden window. I have to restart Firefox four or five times before it works. It's kind of random and I assume something to do with the (random?) order things start up when Firefox is launched. Of course, this may be a totally different issue - I've done no real investigation.

Comment 39

[Emilio Cobos Álvarez [:emilio]]

Please file a separate bug for comment 38, please. I filed bug 2003255 for the issue described above.

Comment 40

[Helmut Leininger]

Just to mention: I have this problem (https://bugzilla.mozilla.org/show_bug.cgi?id=2003162) without any extensions installed in Firefox

Comment 41

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 2002481 - Fire async load event for CKEditor 4 wysiwyg editor iframes. r=hsivonen!,smaug!

Comment 42

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/fc50bf838085
https://hg.mozilla.org/integration/autoland/rev/e4cf436d6363
Fire async load event for CKEditor 4 wysiwyg editor iframes. r=hsivonen,smaug

Comment 43

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/10244b682ea3
https://hg.mozilla.org/integration/autoland/rev/700470ec00b4
Use fully qualified type for rooted to appease builds.

Comment 44

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/93abf1e4dc5f
https://hg.mozilla.org/integration/autoland/rev/139dfa7fe8aa
Fix warning message's usage of apostrophes.

Comment 45

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/e4cf436d6363
https://hg.mozilla.org/mozilla-central/rev/700470ec00b4
https://hg.mozilla.org/mozilla-central/rev/139dfa7fe8aa

Comment 46

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #32)

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #30)

If could check some global objects which is specific thing of CKEditor, this hack could work with customized instances which do not use cke_wysiwyg_frame class.

Can you elaborate? When would that happen, if an app forks ckeditor or something? I don't think we should worry about that in practice. Not applying the intervention in that case seems still better than rewriting the script to me?

I don't have any concrete ideas. However, in my experience, some major websites used DraftJS with customizing something for meeting their requirements and they do not maintain the customized DraftJS. So, my comment was a suggestion for somebody getting the better ideas.

Comment 47

[Henri Sivonen (:hsivonen)]

(It looks like CKEditor 3.6.6.1 works without intervention, so this indeed is limited to CKEditor 4, not 3, not 5.)

Comment 48

[Adrian London]

I've just updated to the latest Nightly, and CKEditor 4 is working again.

Thanks very much for such a fast response. It was impressive to see how this bug got actioned and resolved.

Comment 49

[Henri Sivonen (:hsivonen)]

Thank you for reporting and for confirming the fix.

Comment 50

[Henri Sivonen (:hsivonen)]

The plan for 147 is to back bug 543435 out of beta in bug 2003720, so in bug 2003720 I'm flipping the pref that was introduced here.

Comment 51

[Ryan VanderMeulen [:RyanVM]]

The original change that caused this regression was backed out from Fx147 as per comment 50 (and therefore effectively also avoids this issue). This fix remains in place for Fx148.