Closed
Bug 536895
Opened 15 years ago
Closed 15 years ago
Setting a negative maxLength should throw an exception
Categories
(Core :: DOM: Core & HTML, enhancement)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla1.9.3a3
People
(Reporter: ayg, Assigned: mounir)
References
()
Details
(Keywords: dev-doc-complete, html5)
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US) AppleWebKit/532.6 (KHTML, like Gecko) Chrome/4.0.266.0 Safari/532.6
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20091227 Minefield/3.7a1pre
Test case:
data:text/html,<!doctype html><script>
var input = document.createElement('input');
var caught = false;
try {
input.maxLength = -1;
} catch (e) {
caught = true;
}
if (caught) {
document.write('<p>Exception thrown, correct');
} else {
document.write('<p>No exception thrown, bug');
}</script>
WebKit throws an exception, Opera and Firefox do not. Didn't test IE.
Spec references:
<http://www.whatwg.org/specs/web-apps/current-work/multipage/the-input-element.html#dom-input-maxlength>
says "The maxLength IDL attribute must reflect the maxlength content attribute, limited to only non-negative numbers." The last phrase is a link to
<http://www.whatwg.org/specs/web-apps/current-work/multipage/urls.html#limited-to-only-non-negative-numbers>
which says "On setting, if the value is negative, the user agent must fire an INDEX_SIZE_ERR exception."
This is a follow-up to bug 535043.
Reproducible: Always
Updated•15 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Updated•15 years ago
|
Assignee: nobody → mounir.lamouri
Severity: minor → enhancement
Status: NEW → ASSIGNED
Keywords: html5
Version: unspecified → Trunk
Assignee | ||
Comment 1•15 years ago
|
||
This patch is following patch from bug 536891 by throwing an exception if the maxLength is set to a negative value from the DOM.
Attachment #428420 -
Flags: review?(Olli.Pettay)
Comment 2•15 years ago
|
||
Would it make sense to add some helper macro to nsGenericHTMLElement
Maybe NS_IMPL_UNSIGNED_INT_ATTR or NS_IMPL_NON_NEGATIVE_INT_ATTR?
Assignee | ||
Comment 3•15 years ago
|
||
The only problem I see with a macro is the exception has to be thrown when the attribute is represented as a long in the IDL and should be non negative (like maxLength). If the attribute is represented an unsigned long in the IDL, when trying to set a non negative attribute, the behavior is different.
I think NS_IMPL_NON_NEGATIVE_INT_ATTR would be appropriate if a macro NS_IMPL_NON_NEGATIVE_UNSIGNED_INT_ATTR is used in the future for the other case. Actually, ParseNonNegativeUnsignedIntValue will also have to be created.
Does it make sense for you ?
Comment 4•15 years ago
|
||
Comment on attachment 428420 [details] [diff] [review]
Patch v0.1
I assume there will be a new patch coming :)
Attachment #428420 -
Flags: review?(Olli.Pettay)
Comment 5•15 years ago
|
||
> Actually, ParseNonNegativeUnsignedIntValue will also have to be created.
That is something for a different bug and all. If it will be needed, then it will be.
Assignee | ||
Comment 6•15 years ago
|
||
Creating and using NS_IMPL_NON_NEGATIVE_INT_ATTR macro.
Attachment #428420 -
Attachment is obsolete: true
Attachment #428441 -
Flags: review?(Olli.Pettay)
Updated•15 years ago
|
Attachment #428441 -
Flags: review?(Olli.Pettay) → review+
Comment 7•15 years ago
|
||
Comment on attachment 428441 [details] [diff] [review]
Patch v0.2
>+ * A macro to implement the getter and setter for a given content
>+ * property that needs to set a non-negative integer. The method
>+ * uses the generic GetAttr and SetAttr methods. This macro is much
>+ * like the NS_IMPL_INT_ATTR macro except we throw an exception if
>+ * the set value is negative.
>+ */
>+#define NS_IMPL_NON_NEGATIVE_INT_ATTR(_class, _method, _atom) \
Some extra spaces before \
(No need to ask re-review)
Assignee | ||
Comment 8•15 years ago
|
||
jst, may you sr this patch ? It is following patch from bug 536891.
Attachment #428441 -
Attachment is obsolete: true
Attachment #428738 -
Flags: superreview?
Assignee | ||
Updated•15 years ago
|
Attachment #428738 -
Flags: superreview? → superreview?(jst)
Assignee | ||
Updated•15 years ago
|
Keywords: dev-doc-needed
Updated•15 years ago
|
Attachment #428738 -
Flags: superreview?(jst) → superreview+
Assignee | ||
Comment 9•15 years ago
|
||
Marking checkin-needed with r=Olli.Pettay, sr=jst
Keywords: checkin-needed
Comment 11•15 years ago
|
||
After pushing Bug 536891, this should apply with some fuzz
Keywords: checkin-needed
Assignee | ||
Comment 12•15 years ago
|
||
I've just generated a new patch. It should be easier to apply.
Please, apply the patch from bug 536891 before.
Attachment #428738 -
Attachment is obsolete: true
Comment 13•15 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Updated•15 years ago
|
Keywords: checkin-needed
Target Milestone: --- → mozilla1.9.3a3
Comment 14•15 years ago
|
||
OK, in combination with bug 536891, is the new behavior that setting maxlength to a negative number sets it to unspecified length and throws an exception both, or...?
Assignee | ||
Comment 15•15 years ago
|
||
If you do :
input.maxLength = -2;
it will throw an exception and the maxLength will not change.
If you do:
input.setAttribute('maxLength', -10);
it will work but when you will get it with:
input.maxLength
you will get -1 (as unspecified).
Comment 16•14 years ago
|
||
Updated the doc here:
https://developer.mozilla.org/en/HTML/Element/Input
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•