Last Comment Bug 536895 - Setting a negative maxLength should throw an exception
: Setting a negative maxLength should throw an exception
Status: RESOLVED FIXED
: dev-doc-complete, html5
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: mozilla1.9.3a3
Assigned To: Mounir Lamouri (:mounir)
:
Mentors:
http://dev.w3.org/html5/spec/infrastr...
Depends on:
Blocks: html5forms 535043
  Show dependency treegraph
 
Reported: 2009-12-27 12:58 PST by :Aryeh Gregor (working until September 2)
Modified: 2010-08-18 14:01 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v0.1 (8.58 KB, patch)
2010-02-23 03:46 PST, Mounir Lamouri (:mounir)
no flags Details | Diff | Splinter Review
Patch v0.2 (9.49 KB, patch)
2010-02-23 06:58 PST, Mounir Lamouri (:mounir)
bugs: review+
Details | Diff | Splinter Review
Patch v0.2.1 (9.61 KB, patch)
2010-02-24 09:03 PST, Mounir Lamouri (:mounir)
jst: superreview+
Details | Diff | Splinter Review
Patch v0.2.2 (9.66 KB, patch)
2010-03-05 11:00 PST, Mounir Lamouri (:mounir)
no flags Details | Diff | Splinter Review

Description :Aryeh Gregor (working until September 2) 2009-12-27 12:58:17 PST
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
Comment 1 Mounir Lamouri (:mounir) 2010-02-23 03:46:24 PST
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.
Comment 2 Olli Pettay [:smaug] (vacation Aug 25-28) 2010-02-23 04:41:35 PST
Would it make sense to add some helper macro to nsGenericHTMLElement
Maybe NS_IMPL_UNSIGNED_INT_ATTR or NS_IMPL_NON_NEGATIVE_INT_ATTR?
Comment 3 Mounir Lamouri (:mounir) 2010-02-23 04:56:16 PST
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 Olli Pettay [:smaug] (vacation Aug 25-28) 2010-02-23 05:04:15 PST
Comment on attachment 428420 [details] [diff] [review]
Patch v0.1

I assume there will be a new patch coming :)
Comment 5 Olli Pettay [:smaug] (vacation Aug 25-28) 2010-02-23 05:07:20 PST
> 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.
Comment 6 Mounir Lamouri (:mounir) 2010-02-23 06:58:06 PST
Created attachment 428441 [details] [diff] [review]
Patch v0.2

Creating and using NS_IMPL_NON_NEGATIVE_INT_ATTR macro.
Comment 7 Olli Pettay [:smaug] (vacation Aug 25-28) 2010-02-23 07:43:45 PST
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)
Comment 8 Mounir Lamouri (:mounir) 2010-02-24 09:03:09 PST
Created attachment 428738 [details] [diff] [review]
Patch v0.2.1

jst, may you sr this patch ? It is following patch from bug 536891.
Comment 9 Mounir Lamouri (:mounir) 2010-03-02 15:06:18 PST
Marking checkin-needed with r=Olli.Pettay, sr=jst
Comment 10 Dão Gottwald [:dao] 2010-03-04 01:32:14 PST
this patch has bitrotted
Comment 11 Olli Pettay [:smaug] (vacation Aug 25-28) 2010-03-04 02:18:12 PST
After pushing Bug 536891, this should apply with some fuzz
Comment 12 Mounir Lamouri (:mounir) 2010-03-05 11:00:17 PST
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.
Comment 13 Olli Pettay [:smaug] (vacation Aug 25-28) 2010-03-05 12:27:56 PST
http://hg.mozilla.org/mozilla-central/rev/1a99610b2502
Comment 14 Eric Shepherd [:sheppy] 2010-03-15 12:40:37 PDT
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...?
Comment 15 Mounir Lamouri (:mounir) 2010-03-15 13:06:20 PDT
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 Eric Shepherd [:sheppy] 2010-08-18 14:01:17 PDT
Updated the doc here:

https://developer.mozilla.org/en/HTML/Element/Input

Note You need to log in before you can comment on or make changes to this bug.