Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Why do we load ckeditor.css on every page in ckeditor_init()? Why not load it only when necessary?
Comment | File | Size | Author |
---|---|---|---|
#13 | attach-ckeditor-css-1370894-13.patch | 14.38 KB | Devin Carlson |
#9 | attach-ckeditor-css-1370894-9.patch | 13.28 KB | wwalc |
#8 | attach-ckeditor-css-1370894-8.patch | 10.91 KB | Devin Carlson |
#3 | conditionally_load_ckeditor.css-1370894-3.patch | 1.03 KB | mgifford |
#1 | conditionally_load_ckeditor.css-1370894.patch | 1.38 KB | uberhacker |
Comments
Comment #1
uberhacker CreditAttribution: uberhacker commentedAttached is a proposed fix for this issue.
Comment #2
uberhacker CreditAttribution: uberhacker commentedComment #3
mgiffordPatch didn't apply to the git repo. I've re-rolled the patch.
EDIT: The functions moved around a bunch but I put it in
ckeditor_load_toolbar_option()
which I think will work.Using http://gtmetrix.com to review my site it looks like a bunch of this is really just useless for anonymous users:
Comment #4
Devin Carlson CreditAttribution: Devin Carlson commentedA patch to split the current CSS up into three separate files (to prevent useless CSS from being loaded for certain users, as @mgifford mentioned in #3) and properly
#attach
them to form items/render arrays.Comment #5
kaipee_s2s CreditAttribution: kaipee_s2s commentedDoes the patch (#4) apply conditions around the loading of the css files so that they are only requested when CKEditor is actually loaded?
For example on my home page the ckeditor.css file gets loaded - but there isn't even any input field and CKeditor does not load.
In fact, I have 72 css files and 32 scripts being loaded, on average, on each page - most of which don't need to be (such as this ckeditor css file).
Comment #6
Devin Carlson CreditAttribution: Devin Carlson commented@kaipee_s2s Yes, the patch in #4 is to change CKEditor to only load CSS when necessary.
You can help test by applying the patch and checking whether the CKEditor CSS is only added where appropriate: when the page has a text_format, when the text_format is configured to load the editor and when you're editing the CKEditor settings on the administration page.
Comment #7
Devin Carlson CreditAttribution: Devin Carlson commentedA reroll of #4.
Comment #8
Devin Carlson CreditAttribution: Devin Carlson commentedAn updated patch to add the missing files.
Comment #9
wwalc CreditAttribution: wwalc commented@Anyone interested in having this committed: please let me know if the #9 patch makes sense.
Few notes:
1) ckeditor.css should be loaded on every single page, unfortunately, because it contains definitions of classes that could have been used to create content in the past. If user created a page where alignment was done with
.rteleft
,rteindent1
, then this CSS file is critical in order to have the content displayed properly as before, and we're not talking here in the "edit mode" but simply when the page is rendered.2) Actually ACF for CKEditor should be configured by default in this module to allow such classes like
.rteleft
,rteindent1
etc., considering that config.indentClasses are defined in ckeditor.config.js, but I'll handle this in a separate issue.3) Comparing to the patch in #8:
4) I was feeling really bad whenever I saw that big ugly CSS loaded on every page. I'm happy to see the size of the file being reduced to the absolute minimum.
Comment #10
loopduplicate9: attach-ckeditor-css-1370894-9.patch queued for re-testing.
Comment #11
loopduplicateThe patch didn't seem to apply cleanly for me after this morning's commit to the dev branch so I'm queuing it for retesting.
Comment #13
Devin Carlson CreditAttribution: Devin Carlson commentedA reroll of #9 that #attaches ckeditor.css to the page in
hook_page_build()
for improved cacheability and alterability. It also sets'every_page' => TRUE
so that the CSS gets aggregated with any other CSS that belongs on every page.Comment #15
wwalc CreditAttribution: wwalc commentedThank you Devin and everyone involved, the patch looks cool, so I pushed the changes. Hurray :)
Comment #17
mchaplin CreditAttribution: mchaplin commentedHi
Did this change ever make it into a normal release?
We have version 7.x-1.17.
Our home page references sites/all/modules/ckeditor/css/ckeditor.css
Thanks
Mike