Automatically prevent CKEditor from loading from the CDN when far future functionality is enabled

Why is this necessary? For nearly 2000 sites this hasn't been a blocker to use it.

Could you please export and share your cdn.settings configuration? 🙏

Hm, those are CORS errors. So this appears to be related to but different from #2811615: [upstream] [Core bug, fixed since 9.5] AJAX commands that need additional JS to be loaded will fail when JS is loaded from CDN. See the screenshot and screencast there, you'll see the similarity.

Did you configure Drupal core's CORS support?

Hi, I got the same kind of bug, I have cors activated.
With farfuture activated, ckeditor wasn't loading properly in node/edit/xxx pages.
When inspecting using the console, I got a 403.
To be sure it wasn't cors related, I asked the ckeditor files directly (ie : by pasting the link in my browser bar) => 403 (full themed page "you are not allowed").
I've end up enabling cdn + farfuture for all except js+css

Here is my cdn.settings

langcode: fr
status: true
mapping:
#  type: complex
#  fallback_domain: static1.lequotidiendumedecin.fr
#  domains:
#  -
#    type: auto-balanced
#    domains:
#      - static-css.lequotidiendumedecin.fr
#      - static-js.lequotidiendumedecin.fr
#    conditions:
#      extensions: [css, eot, otf, woff, woff2, ttf, js]
#  -
  type: auto-balanced
  domains:
     - static1.lequotidiendumedecin.fr
     - static2.lequotidiendumedecin.fr
     - static3.lequotidiendumedecin.fr
     - static4.lequotidiendumedecin.fr
     - static5.lequotidiendumedecin.fr
     - static6.lequotidiendumedecin.fr
     - static7.lequotidiendumedecin.fr
     - static8.lequotidiendumedecin.fr
     - static9.lequotidiendumedecin.fr
  conditions:
    extensions:
       - gif
       - jpg
       - jpeg
       - png
       - pdf
       - svg
farfuture:
  status: true
stream_wrappers:
  - public

and my cors settings in services.yml

  cors.config:
    enabled: true
    # Specify allowed headers, like 'x-allowed-header'.
    allowedHeaders: ['x-csrf-token','authorization','content-type','accept','origin','x-requested-with', 'access-control-allow-origin','x-allowed-header']
    # Specify allowed request methods, specify ['*'] to allow all possible ones.
    allowedMethods: ['*']
    # Configure requests allowed from specific origins.
    allowedOrigins: ['*']
    # Sets the Access-Control-Expose-Headers header.
    exposedHeaders: false
    # Sets the Access-Control-Max-Age header.
    maxAge: 60
    # Sets the Access-Control-Allow-Credentials header.
    supportsCredentials: false

David

Hi forgot to mention, that I've started to debug it, and the 403 was fired by CdnFarfutureController here :

   // Validate security token.
    $relative_file_url = $request->query->get('relative_file_url');
    $calculated_token = Crypt::hmacBase64($mtime . $scheme . $relative_file_url, $this->privateKey->get() . Settings::getHashSalt());
    if ($security_token !== $calculated_token) {
      throw new AccessDeniedHttpException('Invalid security token.');
    }

The calculated token was different from the security token.

David

Hi, I've set this issue to active, because I have provided more info.

Ahhh … right. CKEditor 4 does some really weird stuff with automatic asset loading. It makes assumptions that prevents us from applying far future caching optimizations because those involve embedding information in the URL.

This patch should solve the problem — please test and report back!

A similar problem was reported at #2849041: CKEditor translation JS files cannot be loaded (403) when using Interface Translation + "Serve all files" + "Forever cacheable files", #2915417: 403 due to invalid security token has terrible DX (was: "CKEditor / X-Content-Type-Options") and #3033066: 403 error for font files in ckeditor style sheet, but I did not see this potential work-around back then.

This is even the reason I did #2827998: Add a new default option to the CDN UI: "all files except CSS+JS", and make this the new default of the CDN module, include upgrade path … which means that in the 8.x-4.x version of the CDN module, it would become feasible to change the default back to "all files". Although the reasoning in #2827998 for not doing that still stands.

P.S.: the work-around provided in the patch in #11 is ugly technically speaking, but it's the right thing to do: not every user should have to figure this out, not everybody should have to understand the weirdness of CKEditor 4's automatic asset loading.

Fix coding standards violation.

Hello,

CKEditor on admin pages breaks when CDN is enabled.
Also some of the drupal core ajax functionality doesn't work properly when CDN is enabled.

Keeping this points in mind, i have created a patch that will exclude JS and CSS using CDN on admin pages.

@gauravmanerkar Rather than posting a new patch, could you please test the patch in #13 and provide feedback? 😊Thanks! 🙏

Just came across this issue.
Patch works for me.
When reviewing the changes some questions came to mind:

1. +++ b/cdn.module
   @@ -6,6 +6,7 @@
   +use Drupal\editor\Entity\Editor;
   
   @@ -56,3 +63,32 @@ function cdn_file_url_alter(&$uri) {
   +      $editor = Editor::load($format);
   

   Shouldn't we avoid to use "use Drupal\editor\Entity\Editor" as it is a potentially optional component we don't have a dependency on?

2. +++ b/cdn.module
   @@ -56,3 +63,32 @@ function cdn_file_url_alter(&$uri) {
   +  if (!\Drupal::service('cdn.settings')->farfutureIsEnabled()) {
   

   Add check if editor module is enabled?

#16:

Thank you so much for testing, and confirming that the patch works for you! :) That means this can go to RTBC 🥳

At first I strongly agreed, I started to change the code, but then I realized that it's a non-issue, because: hook_editor_js_settings_alter() 😀

What is a problem is this:

$ckeditor_plugin_manager = \Drupal::service('plugin.manager.ckeditor.plugin');

A site may use a different text editor than CKEditor, and this code would fail.

Thank you so much for testing, and confirming that the patch works for you!

No, thank you for the module and the patch :D

At first I strongly agreed, I started to change the code,

I just wasn't entirely sure if use on a missing namespace might causes trouble but it seems that it only matters when a class in that missing namespace would be accessed. So I totally agree, thanks to the module specific hook we're good.

A site may use a different text editor than CKEditor, and this code would fail.

Agreed - and solution looks good :)

Let's get this in!