Setting a negative maxLength should throw an exception

RESOLVED FIXED in mozilla1.9.3a3

Status

()

Core
DOM: Core & HTML
--
enhancement
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: ayg, Assigned: mounir)

Tracking

(Blocks: 1 bug, {dev-doc-complete, html5})

Trunk
mozilla1.9.3a3
dev-doc-complete, html5
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 3 obsolete attachments)

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
Blocks: 535043
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Updated

7 years ago
Assignee: nobody → mounir.lamouri
Severity: minor → enhancement
Status: NEW → ASSIGNED
Keywords: html5
Version: unspecified → Trunk
(Assignee)

Comment 1

7 years ago
Created attachment 428420 [details] [diff] [review]
Patch v0.1

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

7 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

7 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

7 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

7 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

7 years ago
Created attachment 428441 [details] [diff] [review]
Patch v0.2

Creating and using NS_IMPL_NON_NEGATIVE_INT_ATTR macro.
Attachment #428420 - Attachment is obsolete: true
Attachment #428441 - Flags: review?(Olli.Pettay)

Updated

7 years ago
Attachment #428441 - Flags: review?(Olli.Pettay) → review+

Comment 7

7 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)

Updated

7 years ago
Blocks: 344614
(Assignee)

Comment 8

7 years ago
Created attachment 428738 [details] [diff] [review]
Patch v0.2.1

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

7 years ago
Attachment #428738 - Flags: superreview? → superreview?(jst)
(Assignee)

Updated

7 years ago
Keywords: dev-doc-needed

Updated

7 years ago
Attachment #428738 - Flags: superreview?(jst) → superreview+
(Assignee)

Comment 9

7 years ago
Marking checkin-needed with r=Olli.Pettay, sr=jst
Keywords: checkin-needed
this patch has bitrotted
Keywords: checkin-needed
After pushing Bug 536891, this should apply with some fuzz
Keywords: checkin-needed
(Assignee)

Comment 12

7 years ago
Created attachment 430672 [details] [diff] [review]
Patch v0.2.2

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
http://hg.mozilla.org/mozilla-central/rev/1a99610b2502
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Updated

7 years ago
Keywords: checkin-needed
Target Milestone: --- → mozilla1.9.3a3
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

7 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).
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.