IDL attributes reflecting non-negative integer content attributes should throw IndexSizeError when set to a negative number

[jdm]

An example is document.createElement('input').size = -5. Firefox throws an exception; Servo silently sets it to -1. https://html.spec.whatwg.org/multipage/infrastructure.html#limited-to-only-non-negative-numbers says that Firefox is right.

We need to modify parse_plain_attribute to return a Result<AttrValue, Error>, and take an argument indicating whether the attribute is being set by the parser or not. If it is, the default value should be used; if it's not, an Err(IndexSizeError) value should be returned instead.

Code: components/script/dom/element.rs, components/script/dom/virtualmethods.rs

[jdm]

I was reading the code wrong. This is accomplished by make_limited_uint_getter.

[jdm]

Actually a real problem:

<input id="foo" size=-5>
<script>alert(document.getElementById('foo').size); var i = document.createElement('input'); i.size = -5; alert(i.size)</script>

ALERT: 20
ALERT: 20

[jdm]

The first correctly fails silently and yields the default value. The second should throw an IndexSizeError exception.

[nox]

Your link is for long attributes:

If a reflecting IDL attribute has a signed integer type (long) that is limited to only non-negative numbers then, (…)

HTMLInputElement.size is actually unsigned long:

On setting, if the value is zero, the user agent must throw an IndexSizeError exception. Otherwise, first, if the new value is in the range 1 to 2147483647, then let n be the new value, otherwise let n be the default value, or 1 if there is no default value; then, n must be converted to the shortest possible string representing the number as a valid non-negative integer and that string must be used as the new content attribute value.

See #5923, web-platform-tests/wpt#1802, AFAICT the attribute you mention is already covered in WPT, with all its associated edge cases.

Edit: fixed quote for unsigned long.

[jdm]

And the testcase from above?

[nox]

This is the intended behaviour AFAIK. Let me try to reread the tests code and see where the hell that's actually tested.

[jdm]

Oh right, my mistake. Firefox behaves the same as Servo. This is more relevant for #7323.

[nox]

servo/tests/wpt/web-platform-tests/html/dom/reflection.js

Lines 422 to 462 in 576aad5

	/**
	* "If a reflecting IDL attribute is an unsigned integer type (unsigned
	* long) that is limited to only non-negative numbers greater than zero,
	* then the behavior is similar to the previous case, but zero is not
	* allowed. On getting, the content attribute must first be parsed
	* according to the rules for parsing non-negative integers, and if that is
	* successful, and the value is in the range 1 to 2147483647 inclusive, the
	* resulting value must be returned. If, on the other hand, it fails or
	* returns an out of range value, or if the attribute is absent, the
	* default value must be returned instead, or 1 if there is no default
	* value. On setting, if the value is zero, the user agent must fire an
	* INDEX_SIZE_ERR exception. Otherwise, the given value must be converted
	* to the shortest possible string representing the number as a valid
	* non-negative integer and then that string must be used as the new
	* content attribute value."
	*/
	"limited unsigned long": {
	"jsType": "number",
	"defaultVal": 1,
	"domTests": [minInt - 1, minInt, -36, -1, 0, 1, maxInt,
	maxInt + 1, maxUnsigned, maxUnsigned + 1, "", "-1", "-0", "0", "1",
	"\u00097", "\u000B7", "\u000C7", "\u00207", "\u00A07", "\uFEFF7",
	"\u000A7", "\u000D7", "\u20287", "\u20297", "\u16807", "\u180E7",
	"\u20007", "\u20017", "\u20027", "\u20037", "\u20047", "\u20057",
	"\u20067", "\u20077", "\u20087", "\u20097", "\u200A7", "\u202F7",
	"\u30007",
	" " + binaryString + " foo ", undefined, 1.5, true, false,
	{"test": 6}, NaN, +Infinity, -Infinity, "\0",
	{toString:function() {return 2;}, valueOf: null},
	{valueOf:function() {return 3;}}],
	"domExpected": function(val) {
	var parsed = ReflectionTests.parseNonneg(String(val));
	// Note maxInt, not maxUnsigned.
	if (parsed === false || parsed < 1 || parsed > maxInt) {
	return null;
	}
	return parsed;
	},
	"idlTests": [0, 1, 2147483647],
	"idlDomExpected": [null, 1, 2147483647]
	},

for the reference, and because I don't want to believe I looked around for that for nothing. :P