670922 - Add support for read-only attributes in reflect.js reflectUnsignedInt()

Status: RESOLVED WONTFIX

(Core :: DOM: Core & HTML, enhancement)

Description

[Mats Palmgren (inactive)]

Add support for read-only attributes in reflect.js reflectUnsignedInt()

This is needed for the test in bug 648910.

Comment 1

[Mats Palmgren (inactive)]

Attachment: patch v1

Comment 2

[Mounir Lamouri (:mounir)]

Comment on attachment 545378 [details] [diff] [review]
patch v1

Review of attachment 545378 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the nits fixed.

::: content/html/content/test/reflect.js
@@ +122,5 @@
>   *  - element           Element   node to test on
>   *  - attribute         String    name of the attribute
>   *  - nonZero           Boolean   whether the attribute should be non-null
>   *  - defaultValue      Integer   [optional] default value, if different from the default one
> + *  - readOnly          Boolean   whether the attribute is read-only

Could you add [optional] in the description.

@@ +130,5 @@
>    var element = aParameters.element;
>    var attr = aParameters.attribute;
>    var nonZero = aParameters.nonZero;
>    var defaultValue = aParameters.defaultValue;
> +  var readOnly = aParameters.readOnly;

Could you do:
var readOnly = aParameters.readOnly === true;

Comment 3

[Mounir Lamouri (:mounir)]

Comment on attachment 545378 [details] [diff] [review]
patch v1

Review of attachment 545378 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the nits fixed.

::: content/html/content/test/reflect.js
@@ +130,5 @@
>    var element = aParameters.element;
>    var attr = aParameters.attribute;
>    var nonZero = aParameters.nonZero;
>    var defaultValue = aParameters.defaultValue;
> +  var readOnly = aParameters.readOnly;

I've been too fast here: you are misusing defaultValue. You should probably pass the value instead of assuming it's the defaultValue. For that, you can pass an object to readOnly containing a 'value' attribut. Like:

var readOnly = aParameters.readOnly === Object;
if (readOnly) {
  defaultValue = aParameters.readOnly.value;
}

Comment 4

[Mats Palmgren (inactive)]

> you are misusing defaultValue

I don't understand; on (the existing) line 148 we have:

  is(element[attr], defaultValue, "default value should be " + defaultValue);

so the test already makes that assumption.  Why introduce a second mechanism
of passing in defaultValue when we already have one?  Are you saying that the
assumption on line 148 is wrong?

Comment 5

[Mats Palmgren (inactive)]

It seems there isn't any interest in this...