Re: Request for review/approval of reflection tests

Thanks for your review!

On Mon, Feb 21, 2011 at 10:23 PM, Kris Krueger <krisk@microsoft.com> wrote:
> Can you move these into Hg?  That would be a good start and should help get feedback on your tests.

Sorry, I gave the wrong link.  They are in Hg:

http://dvcs.w3.org/hg/html/raw-file/tip/tests/submission/AryehGregor/reflection/reflection-onepage.html

Note that if you're looking into the actual test failures, this
version is probably better:

http://dvcs.w3.org/hg/html/raw-file/tip/tests/submission/AryehGregor/reflection/reflection-original.html

It tests the same things, but it doesn't abort tests on the first
failure.  I could trivially adjust the -onepage version to do that
too, if people think it would be better, but then it's tens of
thousands of separate tests and it's hard to follow the output.  The
-original version groups the failures so you can more easily spot
specific bugs.

> Also I'd rather see that the vast majority of tests for attributes align to real use cases.

We should be testing every requirement in the spec if possible.
Otherwise browsers end up diverging in corner cases.  While it's true
that any given corner case is unlikely to come up on any particular
page, a complicated page is likely to hit several corner cases of some
variety, and the only way to ensure it works the same in all browsers
is to just test all corner cases.  This is the kind of thing that
forces less popular browsers to reverse-engineer more popular ones to
figure out what little quirk of their behavior is causing the page to
behave differently.

> Else we'll end up with a tons of tests for the same bug over and over.

Why is that a problem?  The goal should be 100% pass for all browsers,
so it doesn't matter if one bug causes many test failures.  Repetition
in the reporting can be cut down greatly by processing the test
output.  The output of my reflection-original.html version is pretty
readable in all the browsers I've tried in, despite the huge number of
tests.

> For example tabIndex seems to test -36 and also seem to test 2147483647
> I don't think 2147483647 is a reasonable tabIndex value, even 32767 seems quite an unreasonable value for tabIndex (who is really going to hit tab 32K times?).

If we want tabindex to behave differently from other integer
attributes, we should write that clearly in the spec.  That's fine by
me.  But as long as the spec requires particular behavior in these
cases, we should test it.

I just tested various browsers.  The behavior of

<!doctype html>
<script>
var el = document.createElement("input");
el.tabIndex = 32768;
alert(el.tabIndex);
</script>

is that Firefox 4b11 and Chrome dev alert "32767" (clamping), IE9 RC
and Opera 11 alert "32768".  If I do 65536 instead of 32768, then
Firefox 4b11 and Chrome dev still alert "32767", and Opera 11 alerts
"65536", but IE9 RC alerts "0".  Further experimentation shows that
IE9 RC treats tabIndex as an unsigned short, but doesn't allow
focusing elements with tabIndex above 32767.  This means that in the
fairly common case (see below) of <input tabindex=-1>, IE9 will report
the value in .tabIndex as 65535 while all other browsers will report
-1, which seems like an IE bug that should be fixed.

It does seem like we have agreement (except from Opera) that tabindex
should be a short, not a long.  This would make it the only short in
all of HTML5, though, so unless it's a problem for browsers to change
it to a long, the spec's behavior seems preferable.  If you think it
should be changed to a short in the spec, I encourage you to file a
spec bug.

> If the spec allows a negative tabIndex that should be a spec bug.

Actually, the spec *requires* a tabIndex of -1 for elements that
aren't focusable:

"The tabIndex IDL attribute must reflect the value of the tabindex
content attribute. Its default value is 0 for elements that are
focusable and −1 for elements that are not focusable."
http://www.whatwg.org/specs/web-apps/current-work/multipage/editing.html#dom-tabindex

It also explicitly allows negative tabindexes to be specified:

"""
The tabindex attribute, if specified, must have a value that is a valid integer.

. . .

If the value is a negative integer

The user agent must allow the element to be focused, but should not
allow the element to be reached using sequential focus navigation.
"""
http://www.whatwg.org/specs/web-apps/current-work/multipage/editing.html#attr-tabindex

> Note that I seems that you can set this attribute it doesn't actually work in any browser that I tried (IE, Chrome, Firefox, Opera)
>
> <!doctype html>
> <button tabIndex=10>test</button>
> <button tabIndex=-11>test</button>
> <button tabIndex=-12>test</button>

All the browsers I tested (IE9 RC, Firefox 4b11, Chrome dev, Opera 11)
followed the spec here: the elements with negative tabindexes weren't
focusable by repeatedly hitting Tab.  This seems like useful
functionality, and anyway we have interoperability on the function of
small negative tabindexes, so I don't think the spec or tests should
be changed.

Thanks again for your feedback.

Received on Tuesday, 22 February 2011 18:54:24 UTC