Ticket #7966 (closed Bug: fixed)

Opened 3 years ago

Last modified 3 years ago

Error suggestion not correct when we enter invalid values for width & height fields in Table dialog

Reported by: satya Owned by: wwalc
Priority: Normal Milestone: CKEditor 3.6.1
Component: General Version: 3.6.1
Keywords: IBM Discussion Cc: damo, jamescun, tmonahan, Anna

Description

To reproduce the defect:

In the Table Properties enter invalid values(-20 or 35ex) for width and press OK button.

Expected Result: An error dialog comes up with a waring message width must be a positive number greater than zero and valid CSS units are px em % .

Actual Result: An error dialog comes up with a waring message Specified value must be a valid CSS length unit

same issue when we enter invalid value for Height

This is not satisfying the criteria for A11y Checkpoint 3.3

Attachments

7966.patch (957 bytes) - added by garry.yao 3 years ago.
7966_2.patch (4.5 KB) - added by wwalc 3 years ago.
7966_3.patch (4.5 KB) - added by wwalc 3 years ago.

Change History

comment:1 Changed 3 years ago by damo

Just to clarify the issues here:

  1. The field with the error should be identified in the message to the user. This can be automatically determined by appending the label of the field to the error message.
  2. "Valid CSS length unit" is not clear enough to the ordinary user. If the format of the field is known by the application, then a clear explanation should be provided. Since there is a known finite list of units accepted, the application is able to provide the valid units to the user.

So a possible suggestion on changing this error message could be: "Width value must be a positive number and can be followed by a valid CSS measurement unit. Valid CSS units are; px, %, in, cm, mm, em, ex, pt and pc", e.g. "100%".

comment:2 Changed 3 years ago by j.swiderski

  • Status changed from new to confirmed

True from rev [6994]

comment:3 Changed 3 years ago by wwalc

  • Milestone set to CKEditor 3.6.1

Changed 3 years ago by garry.yao

comment:4 Changed 3 years ago by garry.yao

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

For me the error message is too long, it would be better if we could keep a shorter link which refers to the CSS spec for references, but looks like we're limited by browser alert ;(

comment:5 Changed 3 years ago by Saare

  • Status changed from review to review_failed
  • Cc Anna added

You've got confused thinking that htmlLength refers to height while cssLength refers to width ;)
I'm calling in Anna for help.

comment:6 Changed 3 years ago by Anna

I would suggest something like this:

invalidCssLength : 'Specified value must be a positive number with or without a valid CSS measurement unit (px, %, in, cm, mm, em, ex, pt, or pc).',

invalidHtmlLength : 'Specified value must be a positive number with or without a valid HTML measurement unit (px or %).',

However, if we were to introduce the additional requirement of adding the field name to the message (it makes sense given that the error message is displayed after the user fills in all the dialog window fields and does not really know which one was filled in incorrectly), I'd suggest something like:

'Value specified for the %Field_name% field must be a positive number with or without a valid CSS measurement unit (px, %, in, cm, mm, em, ex, pt, or pc).',

'Value specified for the %Field_name% field must be a positive number with or without a valid HTML measurement unit (px or %)',


where %Field_name% would be the language string for the field label.

comment:7 Changed 3 years ago by garry.yao

  • Owner garry.yao deleted
  • Status changed from review_failed to new

So please just take over this ticket.

Changed 3 years ago by wwalc

comment:8 Changed 3 years ago by wwalc

  • Status changed from new to review
  • Owner set to wwalc

I have shortened the placeholder just to save few bytes.

comment:9 Changed 3 years ago by wwalc

I have updated the patch, because validation in the iframe dialog should actually use validate.htmlLength than validate.cssLength.

Changed 3 years ago by wwalc

comment:10 Changed 3 years ago by alfonsoml

The problem about stating "a valid CSS measurement unit (px, %, in, cm, mm, em, ex, pt, or pc)" is that now the user will have a great confusion about what are those "em, ex, pt, pc", or understanding how physical units (in, cm, mm) are used in a web page.

Is it really necessary to state all the units or could it be simplified to "a valid CSS measurement unit (eg: px, %, ...)"?

How many of you can really explain what is the usage of those units without looking up their definitions?

Personally: I use % only for widths and font sizes, em for margins and paddings, everything else is in pixels.

comment:11 Changed 3 years ago by Saare

  • Status changed from review to review_passed

Anyway the patch itself is a R+, if anyone sees a problem with committing it feel free to R- the ticket.

comment:12 Changed 3 years ago by wwalc

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

Fixed with [7011]. We need to close the release, so let's continue the discussion in #7948, where bringing back the unit selector was proposed.

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