Problem/Motivation
I am running Drupal 9.2.4 and the latest version of ckeditor5. I've noticed some misbehavior of the underline functionality. On the front-end words and sentences are shown underlined while in the backend they are not. On the other hand i am also unable to select for example a single word to underline. I've selected "underlined" in "one underlined" but the whole set is underlined (see admin.png). same for the examples of the two paragraphs.
Steps to reproduce
- created a text format for ckeditor5
- added the underline button to the active toolbar as well as the table button
- created a new article with the ckeditor5 text format
- added a table to the body textfield as well as two paragraphs
- underlined the word underlined in the table as well as in the first paragraph and then the whole second paragraph
-> i am unable to underline only single words ckeditor5 is always extending the scope to the whole sentence it seems as noticed in the table example and the first paragraph (i've also added the html markup of the source view for the particular table part as well as the paragraphs)
Comment | File | Size | Author |
---|---|---|---|
#18 | 3229172-18.patch | 692 bytes | kostyashupenko |
#12 | clarodevtools.png | 666.78 KB | rkoller |
#12 | gindevtools.png | 715.94 KB | rkoller |
#10 | seven.png | 106.44 KB | rkoller |
#10 | claro.png | 114.78 KB | rkoller |
Comments
Comment #2
Wim LeersExcellent find! Will raise this in the meeting we have with the CKE5 team this Thursday! 😊
Comment #3
Wim LeersFYI: We ran out of time before we got to this one. Will discuss it next time.
Comment #4
Wim LeersCould you please report this at https://github.com/ckeditor/ckeditor5/issues? 🙏
(If you don't, that's fine, I'll do so when I get back from vacation!)
Comment #5
rkollerSure. :) The according issue could be found now over here: https://github.com/ckeditor/ckeditor5/issues/10506
Comment #6
Wim LeersLovely, thank you! :)
Comment #7
Wim LeersI can actually not reproduce this 🙈
This does not seem to be reproducible in either CKEditor 5 on https://ckeditor.com/ckeditor-5/demo/#document nor in Drupal.
Which CKEditor 5 plugins did you enable?
Comment #8
rkollerhm back then when i initially reported the issue i've added all available buttons to the toolbar (guess that is what you mean with plugins in the context of ckeditor5? - see the three config.pngs about the setup of the text format for ckeditor5 - the only steps applied aside requiring and installing the module). that initial old installation i've deleted meanwhile (reproduced on one or two other installations as well). created a new one right now with the latest dev version. tried in safari 13.1.2, chrome 93.0.4577.82 and Firefox 92.0. all the same result: see output.png
in the current version of ckeditor5 it seems to respect the extent of the selection and isn't expanding the underlining to the whole sentence anymore. was able to underline single words as well as several at once. that seems to work as expected now. :) but i am still unable to see the words underlined in the node edit form. just in the front-end the underlined words are showing as you can see in the attached.png.
p.s. when i use the online ckeditor5 demo you've linked i am able to see the underlining in safari 13.1.2 (ckdemo.png -underlined "dear guest" and welcome)
Comment #9
rkolleroh and and i've noticed you've answered on github as well :) so where to keep the conversation going best? in here or better upstream and shall i copy my answer here to the github issue as well this time?
Comment #10
rkolleri guess i can close the upstream issue on github. had an idea while jogging. so far all screenshot were taken on gin. tried now with claro and seven as well. see the results in claro.png and seven.png
investigated a little more. Claro is using the following
in
css/base/elements.pcss.css
which is the active rule when inspecting the following linewhen using Claro.
Comment #11
Wim Leers#10: Ohhh! So this is a Claro-specific bug? It's basically Claro being too … eager?
Comment #12
rkollerYep it looks like. Created two screenshots on the node edit form. one in gin with gin_toolbar installed and one in claro with the gin_toolbar uninstalled and selected the word "underline" in the first sentence "one word to underline in here." (see gindevtools.png and clarodevtools.png). I'll close the github issue if that is ok? Guess there is no need for attention upstream in that case anymore?
p.s. out of curiosity when you tried to reproduce on your end and were unable to you've used Seven?
Comment #13
Wim Leers+1 :)
I was!
Comment #14
rkolleroki thanks for confirmation, closed the upstream issue... and one brief question in regards of the procedure in the issue queue on d.o, would it make sense to add a "Claro" tag to the issue now for discoverability? refer
Comment #15
Wim LeersSure … and I'm even wondering if we should move this to the Claro issue queue? 😅 Assigning to @lauriii for feedback.
Comment #16
lauriiiI think it would be fine to move this to the Claro issue queue in core 👍
Comment #17
Wim Leers👍 Done!
Comment #18
kostyashupenkoWell, i'm not sure why we need all these:
since all of those are user agent styles. Any reason to:
1. double add those styles?
2. set
text-decoration
tonone
foru, ins
tags?After quick research i found
text-decoration: none;
was initially added from this patchProbably it is mistake and it was skipped by reviewers (because before that comment
text-decoration
was set tounderline
and then it was restored, but withnone
value). So i didn't find any explanations about why it was done like this.I'm currently providing a fix for it.
Comment #19
yash.rode CreditAttribution: yash.rode at Acquia commentedtested on my loacal, The underlined text works perfect on both front end and backend side now.
attached screenshots for the same before and after.
Before
After
Comment #20
yash.rode CreditAttribution: yash.rode at Acquia commentedComment #23
lauriiiThank you @kostyashupenko for the research, that was very useful context! 👍
Committed 9f574ee and pushed to 9.3.x. Also cherry-picked to 9.2.x because Claro is experimental. Thanks!