Problem/Motivation
Out of the box, pasting content from Microsoft Word or Google Docs leads to unexpected span elements.
In ckeditor 4, these span elements are deeply nested. For example, a single h2, even after allowing ckeditor to detect and modify that the content was pasted from Word, leads to:
<h2><span><span><span><span><span><span><span><span>This is Heading 2</span></span></span></span></span></span></span></span></h2>
In ckeditor 5, it's better, but still adds a language attribute that in most cases isn't needed:
<h2>
<span lang="EN-US">This is Heading 2</span>
</h2>
Looking back through the git and issue history, it looks like span was added at #1911884-30: Enable CKEditor in the Standard install profile.
The issue doesn't say, but I asked @quicksketch offline and he said:
I think it was primarily because it was needed for underline styling, since
<u>tags were frowned upon at the time. Underlines are<span class="underline">.
As well, #1322906: Allow span tag by default for language changes? was marked as fixed with no commits to support span tags for marking language.
However, today in both Drupal 9 and Drupal 10:
- The basic text format does not enable the underline button in ckeditor by default.
- The language dropdown is not enabled either.
I've seen Lighthouse call this out as a performance issue on large pages due to the number of DOM nodes on the page. It's very confusing to look at a page as a developer debugging this, because visually the page might be relatively simple and it's easy to assume the problem is in the front-end theme or some JavaScript bug, and not that the content itself is saved this way.
Steps to reproduce
- Install from the standard profile.
- Create a paragraph of text in Google Docs or Word.
- Paste it into an article or page body field.
Proposed resolution
If we remove span from the list of allowed tags, it is completely removed with pasting generating much simpler HTML. This wouldn't prevent adding the tag, or solve pasting from word processors when spans are allowed.
- Remove span from
core/profiles/standard/config/install/filter.format.basic_html.yml. - Ensure that span is added in ckeditor 5 configurations when adding the language selector.
- If span is still required for underlines, do the same thing there.
Remaining tasks
User interface changes
API changes
None.
Data model changes
None.
Release notes snippet
The 'Basic HTML' format provided with the Standard profile no longer allows the <span> tag. In particular, CKEditor 4 is prone to generating many unnecessary <span> tags. This should simplify copying and pasting from MS Word, Google Docs and similar programs into either CKEditor 4 or CKEditor 5 on new installs. Existing installs may want to consider removing the tag from text formats and reviewing existing content.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 3316921-4.patch | 1.49 KB | wim leers |
| #4 | interdiff.txt | 597 bytes | wim leers |
| #2 | 3316921-2.patch | 1.01 KB | wim leers |
Comments
Comment #2
wim leersOh, wow, some epic historical research you did there! 👏
I personally do not recall
<span class="underline">being the reason or that even being preferable over<u>. But it looks like @quicksketch is the person who indeed introduced it over there, so … it's probably true?I will say that
filter_htmlhas always allowed thelanganddirattributes on all tags — see #2549077: Allow the "Limit allowed HTML tags" filter to also restrict HTML attributes, and only allow a small whitelist of attributes by default. But that doesn't mean that<span lang="…">was also always allowed. So … I think you're on to something!P.S.: The paste-from-Docs/Word concern is related to #3254346: [upstream] CKEditor 5 PasteFromOffice plugin should not allow MS Office/Apple Pages/Google Docs-specific classes and styles even when HTML is unrestricted. But this issue stands on its own for sure.
Comment #4
wim leersGreat, the validation logic caught that the Basic HTML text format & editor are now out of sync! 🤓
Let's update the editor too.
Comment #5
longwave+1 - we don't allow generic
<div>so why do we allow generic<span>?Issue summary is extremely thoroughly researched and all rationale makes sense to me. I've personally run into the multiple nested spans issue when handling paste-from-Word content so to have this fixed out of the box will be nice!
Therefore, assuming tests pass, to me #4 is RTBC.
I also think we can make the argument for backporting to 9.5.x/10.0.x given that this only affects new installs - existing sites are unaffected.
Comment #6
wim leers+1
Comment #7
mherchelOMG, I've fought against nested spans in CKEditor for so long. This would be a big +1 for people's editing experience.
Comment #8
deviantintegral commentedGiven no one has jumped up to say "this was a feature", could we call this a bug and get it in 9.4.x? Especially since this only affects new installs. I've realized from working on this that so many previous projects I thought spans were intentionally allowed by the site requirements, and not an artifact of installing with standard. Now, those sites all have the spans in content and cleanup means checking to see if editors have added any intentional attributes.
Comment #9
wim leersNot even bug fixes are allowed in patch releases if they're disruptive, so … I doubt it 🤓😅
Comment #10
longwaveDrupal 9.5 and 10.0 are out in 7 weeks, we can wait that long even if it's considered a bug.
Comment #12
catchThis makes lots of sense, thanks for the archaeology! Also agreed with it being fine during beta.
Committed/pushed to 10.1.x and cherry-picked to 10.0.x, thanks!
Didn't apply to 9.5.x, but it's such a trivial change I removed the line locally and committed there too.
Added a release notes snippet and tagging, I don't think this needs a CR though really.
Comment #14
wim leers#10++
Also, wow, this was an incredibly fast turn-around time!
Comment #15
wim leersRelease note tweaks.
Comment #16
longwaveProbably should go in both sets of release notes?
Comment #17
deviantintegral commentedClarify CKEditor 4 / 5 differences and note about reviewing content in release notes.