Re: Request for Review: single-touch tests; deadline Sept 24

This looks great guys!  A few comments:

1. We still want some test that validates 'instanceof TouchList', right?
 This represents a legitimate bug in WebKit, right?  Or is it OK for
TouchList to not actually be a defined symbol?

2. I agree with Cathy that the large number of tests makes things a
little unwieldy (eg. having 10+ 'Pass: touch point is a Touch object' makes
the output pretty cluttered and hard to skim, similarly there's little
value to repeating over and over that TouchList is missing
identifiedTouch).  But I also don't think we want to combine all the checks
into a single test case (since, as Cathy says, that'll cause errors - like
WebKit's missing identifiedTouch - to halt the whole test).  Perhaps the
best tradeoff here is to just test the properties of a given type once.
 I.e. once we've verified one Touch object, it's really overkill to verify
all the others (I think it's pretty unlikely, for example, that any
implementation would have a 'screenX' on some Touch instances but not
others).  Or maybe we put these behind an 'exhaustive mode' or something.

Alternately, we could be a little fancier and get the best of both worlds
by establishing a baseline for a Touch object once (with one test per
property) - keeping track of which tests pass, and in subsequent tests just
assert that all touches match the same baseline.  Let me know if this
sounds worthwhile and I can code it up.

3. I added tests for some additional things that aren't currently covered
by the test assertions:

   - pageX/pageY is over the expected element (for all three event types)
   - clientX/clientY is correct (give page co-ordinates and scroll position)
      - note we should probably have the test initialize itself to non-zero
      scroll offsets - eg. I've seen several bugs in webkit implementation and
      tests due to assuming scroll position is 0.
   - screenX/screenY is consistent (within our ability to validate)
   - no more than one touchstart is received
   - createTouchList and createTouch work as defined
      - Note that this uncovered a new bug in the WebKit implementation -
      as spec'd createTouchList is supposed to take an array of Touch objects,
      but WebKit doesn't handle this - instead supporting a variable number of
      Touch arguments (as can be seen in the WebKit layout test for
this API<http://code.google.com/searchframe#OAMlx_jo-ck/src/content/test/data/layout_tests/LayoutTests/fast/events/touch/script-tests/document-create-touch-list.js&exact_package=chromium&q=createTouchList&type=cs&l=17>).
       I've filed https://bugs.webkit.org/show_bug.cgi?id=97418 against
      myself to track.

I forked Art's version of the tests on gitHub here:
https://github.com/RByers/WebEvents.  The additions above are
here<https://github.com/RByers/WebEvents/commit/b2717c4925fc5f4a7724ad4e9e59be37d8276857#single-touch.html>and
here<https://github.com/RByers/WebEvents/commit/dc21ef44950fe11f38d91a6b85195c43a3d7e8e5#single-touch.html>.
 Art, if these look OK, do you want to pull them into your copy (or even
have a single repo we all have permission to push to?)

With these few additions, I think we've pretty thoroughly covered this
simple single-touch scenario.  I can see a couple additional things we may
want to test in this scenario:

   - non-zero scroll offset (just add some padding to the top, bottom and
   probably left, then have JS set the scroll position so the page looks the
   same as it does now by default).
   - touch point actually lines up with the user's finger - eg. draw a
   small circle on touchstart/touchend for manual verification?

I'm not sure if either is really worth it though - thoughts?  There are a
couple other single-touch scenarios I could imagine testing (in separate
test case files), eg:

   - impact of preventDefault on scrolling
      - I think many of the details here are implementation dependent
      though - eg. if PD isn't called, do you still get touchmove
events during a
      scroll operation?  It may be hard to write a useful test that isn't
      dependent on at least some implementation details.
   - impact of preventDefault on synthesized mouse events
      - but since these are optional - we'd probably technically have to
      make the test pass if no mouse events are detected at all
   - generation of touchcancel
      - this is so platform dependent that I'm not sure what we'd want here
      really - but a simple test that lets a user confirm it is
possible to get a
      touchcancel may be good.
      - While playing with this in chrome, I discovered an interesting bug
      <http://code.google.com/p/chromium/issues/detail?id=151871>where
      instead of getting touchcancel, I can get into a nasty state where I get
      illegal events.
   - interaction of touch events with iframes
      - this is completely unspecified and up to the UA, right?

I'm not sure which of these are actually worth testing though - thoughts?
 I think I could whip up simple tests for the first three pretty quickly -
but I'm not sure I could make the PD ones completely free of any
implementation detail bias.

And of course we have the mutl-touch scenarios to finish.

Rick

On Thu, Sep 20, 2012 at 6:45 PM, Arthur Barstow <art.barstow@nokia.com>wrote:

> On 9/20/12 5:54 PM, Chan Cathy (Nokia-CIC/Boston) wrote:
>
>> First of all, +1 to continuing.
>>
>> A couple comments on the type-checking code
>> 1. I noticed you used
>>         assert_equals(Object.**prototype.toString.call(ev[**name]),
>> "[object
>> TouchList]", name + " attribute of type TouchList");
>> instead of the I-assume-is-equivalent
>>         assert_true(ev[name] instanceof TouchList, name + " attribute of
>> type TouchList");
>> to check whether the said attribute is a TouchList object. While the
>> latter
>> one results in the "Can't find variable: TouchList" error in WebKit [1],
>> the
>> former works without a hitch. We might be inadvertently masking the WebKit
>> bug if we use the former syntax.
>>
>
> Thanks! I'll take a look at that.
>
>
>  (Note that with the type-checking code in
>> place, the line that initially exposed that WebKit bug (line 134 in your
>> version) can and should be removed, leaving the above line as the only
>> place
>> which would expose the bug.)
>>
>
> And that too.
>
>
>  2. The number of test assertions exploded from 10+ to 80+
>>
>
> Yesterday I checked in an update that has many more tests ;-). I believe
> it tests all of the assertions 1.1.* through 1.5.* (although not
> touchcancel). I haven't worked on the 1.6.* assertions but based on a very
> quick scan I think Matt already covered most if not all of them.
>
> Anyhow, today, Matt ran my Sept 19 version on his implementation and got
> 240/240 Passes! My N9 passes 221 and the 19 failures are all related to
> identfiedTouch.
>
> Sangwhan - would you please run [1] against an Opera implementation?
>
> Are there any other implementations besides Wk/Moz/O?
>
>
>
>  I recognize that this might well just be a matter of style and
>> personal preference.
>>
>
> Agreed and I'm somewhat indifferent although I don't have a problem with
> having a relatively strong correlation between asserts and tests (i.e. lots
> of tests don't bother me).
>
> -Art
>
> [1] <https://raw.github.com/**AFBarstow/WebEvents/master/**
> single-touch.html<https://raw.github.com/AFBarstow/WebEvents/master/single-touch.html>
> >
>
>
>
>

Received on Monday, 24 September 2012 02:33:03 UTC