Ckeditor4: RGBA/HSLA colors are incorrectly encoded

Created on 3 Nov 2020 · 9Comments · Source: ckeditor/ckeditor4

Type of report

Bug

Provide detailed reproduction steps (if any)

Open https://ckeditor.com/latest/samples
Select some text.
Open colordialog and add hsla(150,50%,52%,0.2) color.
Open colordialog again.

Expected result

Color should be converted to the correct hashcode including the alpha channel. Note that on the HTML level we are still keeping the original value, so this conversion shouldn't introduce a regression.

It would be good to find out why we are actually unifying colors to hashcode and if it's not really needed, we could instead use the original color value for every color including rgb/hsl.

Actual result

Color is converted to #rgba(71, 194, 133, 0.2)

Other

The issue is also reproducible with rgba colors.

good first issue colordialog S confirmed bug

Source

jacekbogdanski

All 9 comments

Related to #4333.

f1ames on 3 Nov 2020

It would be good to find out why we are actually unifying colors to hashcode and if it's not really needed, we could instead use the original color value for every color including rgb/hsl.

Maybe it is needed for older IEs since they have only partial support?

f1ames on 3 Nov 2020

Hmmm, that's actually quite interesting, this issue relates to a fact that # is added on the beginning of the color name while #4333 to a fact that # (or any other starting character) is removed. So those seems similar, but in fact, this case is related to displaying color name in color dialog (after reopening). And #4333 to displaying color in color history panel.

Still the fact that one part removes # while the other adds it means those mechanisms are related trying to preserve valid color format.

f1ames on 3 Nov 2020

I suppose we are doing it in at least 2 places (could be wrong). If yes, it would be good to unify it into one function. Playing around with magic strings results in such issues.

Maybe it is needed for older IEs since they have only partial support?

Yeah, looks like, so it would be good to go with hexadecimal values.

jacekbogdanski on 4 Nov 2020

👍1

The issue is not as trivial as it seems. The whole normalization of colors happens in colorbutton plugin (which has… two different methods for normalizing – public method and private function). Both these methods assume that color is a hex and adds # at the beginning. Additionally, colordialog plugin adds # on its own.

My proposal is to unify normalization in colorbutton plugin and use hexes generated by CKEDITOR.tools.color exclusively. The only place where the original name of the color would be displayed is the color dialog. The only downside is the fact that it would change the behaviour of the colorbutton and all dependent plugin (including colordialog), but it seems like a move into the right direction. The whole code would be easier to maintain and more colors would be supported just thanks to better normalization.

Comandeer on 23 Mar 2021

👍1

☝🏻 I strongly agree that a normalisation would be useful and help simplify the whole plugin (actually a few plugins). You are probably aware of it but I want to mention that we also use color dialog in tabletools plugin and the way colors are displayed there is wild (in cell props dialog they are converted to rgb format 🤷🏻 ).