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)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rkoller created an issue. See original summary.

Wim Leers’s picture

Title: The underline functionality is misbehaving » [upstream] The underline functionality is misbehaving
Component: User interface » Code
Status: Active » Postponed
Issue tags: +Needs upstream bugfix

Excellent find! Will raise this in the meeting we have with the CKE5 team this Thursday! 😊

Wim Leers’s picture

FYI: We ran out of time before we got to this one. Will discuss it next time.

Wim Leers’s picture

Could 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!)

rkoller’s picture

Sure. :) The according issue could be found now over here: https://github.com/ckeditor/ckeditor5/issues/10506

Wim Leers’s picture

Lovely, thank you! :)

Wim Leers’s picture

Status: Postponed » Postponed (maintainer needs more info)

I 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?

<p>One word to <u>underline</u> in here</p><figure class="table"><table><tbody><tr><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td></tr><tr><td>&nbsp;</td><td>Please <u>underline</u> <s>me</s></td><td>&nbsp;</td></tr><tr><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td></tr><tr><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td></tr><tr><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td></tr><tr><td>&nbsp;</td><td>&nbsp;</td><td>&nbsp;</td></tr></tbody></table></figure>
rkoller’s picture

FileSize
135.32 KB
36.71 KB
161.42 KB
166.53 KB
57.25 KB

hm 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)

rkoller’s picture

oh 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?

rkoller’s picture

FileSize
114.78 KB
106.44 KB

i 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

u,
ins {
   text-decoration: none;
}

in css/base/elements.pcss.css which is the active rule when inspecting the following line

<u>underline</u>

when using Claro.

Wim Leers’s picture

Title: [upstream] The underline functionality is misbehaving » Underlined text not underlined in Claro
Issue tags: -Needs upstream bugfix

#10: Ohhh! So this is a Claro-specific bug? It's basically Claro being too … eager?

rkoller’s picture

FileSize
715.94 KB
666.78 KB

Yep 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?

Wim Leers’s picture

I'll close the github issue if that is ok? Guess there is no need for attention upstream in that case anymore?

+1 :)

p.s. out of curiosity when you tried to reproduce on your end and were unable to you've used Seven?

I was!

rkoller’s picture

oki 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

Wim Leers’s picture

Assigned: Unassigned » lauriii
Status: Postponed (maintainer needs more info) » Active
Issue tags: +Claro

Sure … and I'm even wondering if we should move this to the Claro issue queue? 😅 Assigning to @lauriii for feedback.

lauriii’s picture

Assigned: lauriii » Unassigned

I think it would be fine to move this to the Claro issue queue in core 👍

Wim Leers’s picture

Title: Underlined text not underlined in Claro » Underlined text in CKEditor 5 not rendered as underlined in Claro
Project: CKEditor 5 » Drupal core
Version: 1.0.x-dev » 9.3.x-dev
Component: Code » Claro theme

👍 Done!

kostyashupenko’s picture

Status: Active » Needs review
FileSize
692 bytes

Well, i'm not sure why we need all these:

...

u,
ins {
  text-decoration: none;
}
s,
strike,
del {
  text-decoration: line-through;
}
big {
  font-size: larger;
}
small {
  font-size: smaller;
}

...

since all of those are user agent styles. Any reason to:
1. double add those styles?
2. set text-decoration to none for u, ins tags?

After quick research i found text-decoration: none; was initially added from this patch

Probably it is mistake and it was skipped by reviewers (because before that comment text-decoration was set to underline and then it was restored, but with none value). So i didn't find any explanations about why it was done like this.

I'm currently providing a fix for it.

yash.rode’s picture

Issue summary: View changes

tested 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
Only local images are allowed.

Only local images are allowed.

After
Only local images are allowed.

Only local images are allowed.

yash.rode’s picture

Status: Needs review » Reviewed & tested by the community

  • lauriii committed 9f574ee on 9.3.x
    Issue #3229172 by kostyashupenko, rkoller, Wim Leers, yash.rode, lauriii...

  • lauriii committed 0720d64 on 9.2.x
    Issue #3229172 by kostyashupenko, rkoller, Wim Leers, yash.rode, lauriii...
lauriii’s picture

Version: 9.3.x-dev » 9.2.x-dev
Status: Reviewed & tested by the community » Fixed

Thank 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!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.