Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#8124 closed Bug (fixed)

Styles field is not validated

Reported by: tmonahan Owned by: garry.yao
Priority: Normal Milestone: CKEditor 3.6.2
Component: Accessibility Version: 3.0.2
Keywords: IBM Cc: damo, satya

Description

On all dialogs that provide an advanced tab, the user can specify CSS styling through the Styles field. However this field is not validated when the user clicks OK. Therefore they can enter fake values and it looks to the user that they are accepted.

To reproduce:

  1. Open the Table dialog.
  2. On the advanced tab, enter an invalid value in the Styles field e.g fakeAttribute
  3. Click OK.

Problem: There is no error warning that the value entered is not correct. When the dialog is opened again, the Styles field is now empty.

An error message should be displayed when invalid content is entered in the Styles field. This is an accessibility requirement.

Attachments (1)

8124.patch (2.4 KB) - added by garry.yao 5 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 5 years ago by j.swiderski

  • Status changed from new to confirmed
  • Version changed from 3.6.1 to 3.0.2

comment:2 Changed 5 years ago by alfonsoml

What is "invalid content" for a style?

I guess that any validation should have the option to choose the CSS Recomendation that it wants to perform the validation against. There are things that are new in CSS3 that didn't exist in CSS 2, so it might not be valid for IE6, but new browsers understand that correctly.

Also, are vendor specific rules valid or not?
Or is "valid" a rule that one browser handles but other browsers don't?

The validation should check if the style make sense? For example setting "color:red" on an image. The Specs do state sometimes that some property is valid only for some elements, example: the CSS 2 recomendation like "caption-side" http://www.w3.org/TR/CSS21/tables.html#caption-position but it says that "color" applies to all the elements.

Of course, it must be able to understand the short-hand properties, but sometimes the browser might change the shorthand to include something extra (it used to happen specially with Mozilla that included its own properties in the shorthand for background), is the generated style valid?

The fact that non-valid styles are discarded could be prevented with #5528, but it doesn't fix the fact that the style itself might not be valid, that requires extra validation and understanding of all the CSS properties and values as requested by this ticket.

comment:3 Changed 5 years ago by tmonahan

  • Component changed from General to Accessibility

comment:4 Changed 5 years ago by garry.yao

I think the reporter doesn't really wish of a full CSS style validation? But just the very basic syntax specified here as perhaps the only reference that the spec mentions of it.

comment:5 Changed 5 years ago by fredck

Agreed. The only think we could do here is syntax validation.

Changed 5 years ago by garry.yao

comment:6 Changed 5 years ago by garry.yao

  • Owner set to garry.yao
  • Status changed from confirmed to review

comment:7 Changed 5 years ago by Saare

  • Status changed from review to review_passed

comment:8 Changed 5 years ago by garry.yao

  • Resolution set to fixed
  • Status changed from review_passed to closed

Fixed with [7176].

comment:9 Changed 5 years ago by garry.yao

  • Milestone set to CKEditor 3.6.2

comment:10 Changed 5 years ago by tmonahan

The fix provided for this does go some way in addressing the issue. However there is still a problem when the syntax is valid, but the actual values specified are not valid CSS.

For example, if the user enters a name value pair that is not valid css (e.g. myFakeAttribute: 200px), the syntax validation is satisfied so the error message does not display. However when the dialog is opened again, the Styles field is now empty. The user has not been told that their content is invalid so they have no idea why their content has been removed. This would also occur if the user accidentally mis-spelled a CSS attribute name e.g. widtth: 100px. This is not reported as invalid, but it is removed from the Styles field on the dialog.

Is there anyway to use the same validation that removes these entries from the Styles field, to also alert the user to invalid content when they click OK on the dialog? Do you know at what point the invalid entries get removed from the Styles field? Is this something CKEditor has control over or is it due to the browser?

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