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)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a3

People

(Reporter: ayg, Assigned: mounir)

References

()

Details

(Keywords: dev-doc-complete, html5)

Attachments

(1 file, 3 obsolete files)

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
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: nobody → mounir.lamouri
Severity: minor → enhancement
Status: NEW → ASSIGNED
Keywords: html5
Version: unspecified → Trunk
Attached patch Patch v0.1 (obsolete) (deleted) — Splinter Review
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)
Would it make sense to add some helper macro to nsGenericHTMLElement Maybe NS_IMPL_UNSIGNED_INT_ATTR or NS_IMPL_NON_NEGATIVE_INT_ATTR?
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 on attachment 428420 [details] [diff] [review] Patch v0.1 I assume there will be a new patch coming :)
Attachment #428420 - Flags: review?(Olli.Pettay)
> 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.
Attached patch Patch v0.2 (obsolete) (deleted) — Splinter Review
Creating and using NS_IMPL_NON_NEGATIVE_INT_ATTR macro.
Attachment #428420 - Attachment is obsolete: true
Attachment #428441 - Flags: review?(Olli.Pettay)
Attachment #428441 - Flags: review?(Olli.Pettay) → review+
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)
Blocks: 344614
Attached patch Patch v0.2.1 (obsolete) (deleted) — Splinter Review
jst, may you sr this patch ? It is following patch from bug 536891.
Attachment #428441 - Attachment is obsolete: true
Attachment #428738 - Flags: superreview?
Attachment #428738 - Flags: superreview? → superreview?(jst)
Keywords: dev-doc-needed
Attachment #428738 - Flags: superreview?(jst) → superreview+
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
Attached patch Patch v0.2.2 (deleted) — Splinter Review
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
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
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...?
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).
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: