Opened 5 years ago

Closed 2 years ago

Last modified 2 years ago

#10750 closed Bug (fixed)

The editor don't unquote the font-family style property

Reported by: Leeron Owned by: Marek Lewandowski
Priority: Normal Milestone: CKEditor 4.5.10
Component: General Version: 4.0
Keywords: Support Cc: Oracle

Description

Hopefully I'm missing something here, but here it goes:

I came across a problem where fonts with names like 'My Crazy Sans -32432 W10' doesn't work properly. Looking into it noticed that the font-family is added to the HTML unquoted.

<span style="font-family:my crazy sans -32432 w10;my crazy sans -386932 w02;sans-serif">Some text here.</span>
This inline style property doesn't work on my browser (latest Chrome).

Digging into the code I found the following code at the end of the tools.js file (inside the parseCssText function):

// Normalize font-family property, ignore quotes and being case insensitive. (#7322)
// http://www.w3.org/TR/css3-fonts/#font-family-the-font-family-property
if ( name == 'font-family' )
    value = value.toLowerCase().replace( /["']/g, '' ).replace( /\s*,\s*/g, ',' );

Noticing the (#7322) remark, I went looking for the ticket - ​http://dev.ckeditor.com/ticket/7322.

I guess the editor have a problem with handling quoted font-family values internally. But the spec (that the code and the ticket point to) state clearly that on the HTML (and CSS) the value should be quoted.

And again - it's not only contrast with W3 recommendations, it actually doesn't work on my browser.

From ​http://www.w3.org/TR/CSS2/fonts.html#font-family-prop :

To avoid mistakes in escaping, it is recommended to quote font family names 
that contain white space, digits, or punctuation characters other than hyphens:

body { font-family: "New Century Schoolbook", serif }

<BODY STYLE="font-family: '21st Century', fantasy">

Change History (13)

comment:1 Changed 5 years ago by Wiktor Walc

Keywords: style css font-family quote removed

comment:2 Changed 3 years ago by Jakub Ś

Cc: Oracle added
Status: newconfirmed
Version: 4.0.14.0

First of all we had few tickets describing same issue: #5967, #12492, #5966, #12563 and #7332 (which should fix these issues but created more chaos).

According to http://www.w3.org/TR/CSS2/fonts.html#font-family-prop and http://www.w3.org/TR/css3-fonts/#propdef-font-family: Generic font-family names should not be quoted (thir number is limited)

The following generic family keywords are defined: ‘serif’, ‘sans-serif’, ‘cursive’, ‘fantasy’, and ‘monospace’. These keywords can be used as a general fallback mechanism when an author's desired font choices are not available. As keywords, they must not be quoted. Authors are encouraged to append a generic font family as a last alternative for improved robustness.

while other fonts should/could be quoted

To avoid mistakes in escaping, it is recommended to quote font family names that contain white space, digits, or punctuation characters other than hyphens

additionally:

Font family names that happen to be the same as a keyword value (‘inherit’, ‘serif’, ‘sans-serif’, ‘monospace’, ‘fantasy’, and ‘cursive’) must be quoted to prevent confusion with the keywords with the same names. The keywords ‘initial’ and ‘default’ are reserved for future use and must also be quoted when used as font names. UAs must not consider these keywords as matching the <family-name> type.


First of all I think that that developer and not CKEditor should take care of quoting font names in font_names configuration option. CKEditor should however not lover-cased them (there are case sensitive font names - #12492) or remove quotes around them once added. IMO CKEditor should add font names as they are to dropdowns and elements in content area.
Ticket #12563 as well as this ticket mention parseCssText as the culprit and the comment in that method seems to confirm it. There might be other places where font names are normalized (See e.g. #5966). Anyway they should be the same everywhere (style, font dropdowns and content).

NOTE: It is important to check if the font name is actually left as it was defined because in some cases browsers work well with minified and unquoted font-names.


I have written:

First of all I think that that developer and not CKEditor should take care of quoting font names in font_names configuration option.

Well at first I was thinking that perhaps editor could search for generic fonts and unquote them but that last note from specification blew me of the track thus leaving is to developer to define font names properly might be best approach. We just should not modify anything.

Last edited 3 years ago by Jakub Ś (previous) (diff)

comment:3 Changed 3 years ago by Jakub Ś

#5967, #12492, #5966, #12563 were marked as duplicates.

comment:4 Changed 3 years ago by Jakub Ś

Keywords: Support added

This issue has also been mentioned on our support channel.

comment:5 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.5
Owner: set to Marek Lewandowski
Status: confirmedassigned

We've started investigation on how to fix this issue. Initial solution is already pushed to branch:t/10750 however it need further investigation on how it will impact other parts of CKEditor (e.g. how it will affect CKEDITOR.style based operations).

comment:6 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.5CKEditor 4.5.6

comment:7 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.6CKEditor 4.5.7

comment:8 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.7CKEditor 4.5.8

comment:9 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.8CKEditor 4.5.9

comment:10 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.9CKEditor 4.5.10

comment:11 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.10CKEditor 4.5.11

Moving tickets to the next milestone.

comment:12 Changed 2 years ago by Marek Lewandowski

Resolution: fixed
Status: assignedclosed

We were able to contain it within 4.5.10. Fixed with git:7dc1295fcb (merged to master).

comment:13 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.11CKEditor 4.5.10
Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy