Closed Bug 606817 Opened 9 years ago Closed 9 years ago

Truncate form validation message only if they are content specified

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b7
Tracking Status
blocking2.0 --- beta7+

People

(Reporter: mounir, Assigned: mounir)

References

Details

Attachments

(1 file, 2 obsolete files)

https://developer.mozilla.org/en/Localization_notes

Thanks to Gavin for mentioning that.
Attached patch Patch v1 (obsolete) — Splinter Review
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Attachment #485589 - Flags: review?(l10n)
Should that patch be pushed in b7 branch to make sure localizers will respect the 256 characters limitation or they should be able to fix that in b8? (I've no idea how this comment is shown to them...)
Why are we cutting off at 256 characters to begin with?

Also, I wonder what %S is in FormValidationPatternMismatchWithTitle.

FYI, cutting off is a bad idea in fonts with ligatures. I wonder if we should just cut web-provided strings, and not l10n provided ones.
(In reply to comment #3)
> Why are we cutting off at 256 characters to begin with?

To prevent web content setting a very big string to annoy the user.

> Also, I wonder what %S is in FormValidationPatternMismatchWithTitle.

It adds the text from @title. This is one way for web content to set something in this string.

> FYI, cutting off is a bad idea in fonts with ligatures. I wonder if we should
> just cut web-provided strings, and not l10n provided ones.

I thought about that but it's hard to know which string is returned by .validationMessage (those strings are returned by this property). If there is only one error, it's easy but when there are more numerous, the order is set arbitrarily [1]. IOW, .validationMessage is a black box.
That mean the UI code will have to recode .validationMessage behavior which is error prone given that a change in .validationMessage will break the UI.
There are two other ways for the content to set an error message but when set these ways, .validationMessage will always return the specified string.

[1] http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/nsIConstraintValidation.cpp#76
blocking2.0: --- → beta7+
Why can't the limit be put in the code that adds in the content-specified strings, rather than in the UI?
Attached patch Patch v2 (obsolete) — Splinter Review
Attachment #485589 - Attachment is obsolete: true
Attachment #485716 - Flags: review?(community)
Attachment #485589 - Flags: review?(l10n)
Attachment #485716 - Flags: review?(l10n)
Attachment #485716 - Flags: review?(jonas)
Attachment #485716 - Flags: review?(community)
This patch is truncating content defined messages instead of doing that inside the UI.
Attached patch Patch v2.1Splinter Review
Attachment #485716 - Attachment is obsolete: true
Attachment #485717 - Flags: review?(jonas)
Attachment #485716 - Flags: review?(l10n)
Attachment #485716 - Flags: review?(jonas)
Attachment #485717 - Flags: review?(l10n)
Comment on attachment 485717 [details] [diff] [review]
Patch v2.1

There's one comment left over to truncate the final string.

I'd make the l10n note

%S is the (possibly truncated) title attribute value.
Attachment #485717 - Flags: review?(l10n) → review+
The comment in browser.js also needs updating.
Summary: Add a L10N note in dom.properties about the 256 max characters for strings related to form validation → Truncate form validation message only if they are content specified
Pushed on trunk:
http://hg.mozilla.org/mozilla-central/rev/5796d98f4084

And b7 branch:
http://hg.mozilla.org/mozilla-central/rev/06b654d3950f

Thank you for your reviews :)
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Keywords: dev-doc-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b7
I'm unclear on what if anything actually requires documentation here. This sounds strictly like a behavior change that doesn't really impact anything directly.
Blocks: 610340
No longer blocks: FF2SM
(In reply to comment #12)
> I'm unclear on what if anything actually requires documentation here. This
> sounds strictly like a behavior change that doesn't really impact anything
> directly.

Eric, I guess I've set that in case of the dev-doc was specifying that the error messages (in setCustomValidity for example) shouldn't be longer than 256 characters. If the doc doesn't say that, everything should be fine.
You need to log in before you can comment on or make changes to this bug.