Bug 32492 – Missing support for document.createEvent("OrientationEvent")

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Greg Bolsinga]

When support for OrientationEvents went in with http://trac.webkit.org/changeset/48609, it doesn't have create support.

<rdar://problem/7458132>

[Joseph Pecoraro]

Just some background information: Like other events on the window it can be simulated with createEvent and initEvent:

    var e = document.createEvent("Event");
    e.initEvent('orientationchange', false, false);
    window.dispatchEvent(e);

[Yongjun Zhang]

Created attachment 70795 [details]
Implement document.createEvent("OrientationEvent") to help unit testing onOrientationEvent.

[Yongjun Zhang]

(In reply to comment #2)
> Created an attachment (id=70795) [details]
> Implement document.createEvent("OrientationEvent") to help unit testing onOrientationEvent.

Oops! typo, should be onOrientationChange.

[Yongjun Zhang]

Created attachment 70874 [details]
(2nd try) fix the typo in test description.

[Joseph Pecoraro]

Comment on attachment 70874 [details]
(2nd try) fix the typo in test description.

View in context: https://bugs.webkit.org/attachment.cgi?id=70874&action=review

> LayoutTests/fast/dom/DeviceOrientation/create-event-onorientationchange-expected.txt:11
> +Tests that document.createEvent() works with orientationChange
> +
> +On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
> +
> +
> +
> +
> +
> +PASS successfullyParsed is true
> +
> +TEST COMPLETE

This is rather blank. There is no PASS / FAIL message in the common case.

> LayoutTests/fast/dom/DeviceOrientation/script-tests/create-event-onorientationchange.js:15
> +try {
> +    var event = document.createEvent("OrientationEvent");
> +    event.initEvent("orientationchange", false, false);
> +    window.dispatchEvent(event);
> +} catch(e) {
> +}

I would suggest outputting something in the catch { ... } along the lines of
"Orientation Events don't appear to be enabled or implemented".

I don't know the best way to reduce the overhead of submitting a test which
will only pass behind an ENABLE flag.

[Yongjun Zhang]

(In reply to comment #5)
> (From update of attachment 70874 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=70874&action=review
> 
> > LayoutTests/fast/dom/DeviceOrientation/create-event-onorientationchange-expected.txt:11
> > +Tests that document.createEvent() works with orientationChange
> > +
> > +On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
> > +
> > +
> > +
> > +
> > +
> > +PASS successfullyParsed is true
> > +
> > +TEST COMPLETE
> 
> This is rather blank. There is no PASS / FAIL message in the common case.
> 
> > LayoutTests/fast/dom/DeviceOrientation/script-tests/create-event-onorientationchange.js:15
> > +try {
> > +    var event = document.createEvent("OrientationEvent");
> > +    event.initEvent("orientationchange", false, false);
> > +    window.dispatchEvent(event);
> > +} catch(e) {
> > +}
> 
> I would suggest outputting something in the catch { ... } along the lines of
> "Orientation Events don't appear to be enabled or implemented".
> 
> I don't know the best way to reduce the overhead of submitting a test which
> will only pass behind an ENABLE flag.

thanks for the comments.  Folder fast/dom/DeviceOrientation/ is skipped for most ports, and for those ports (Android?) that enabled device orientation events but not onorientationchange, this test will fail by default, hence the failed result as expected result.   The port need to override the expected result if onorientationchange is implemented.

[Yongjun Zhang]

Created attachment 71051 [details]
Address Joe's comment.

[WebKit Commit Bot]

Comment on attachment 71051 [details]
Address Joe's comment.

Clearing flags on attachment: 71051

Committed r69984: <http://trac.webkit.org/changeset/69984>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Steve Block]

I'm not sure why you've added this test to fast/dom/DeviceOrientation. This LayoutTest directory is to test the implementation of the W3C DeviceOrientationEvent (http://dev.w3.org/geo/api/spec-source-orientation.html, https://bugs.webkit.org/show_bug.cgi?id=30335).

Your change seems to be concerned with the OrientationEvent, which is completely separate.

Furthermore, as you point out, ports that don't implement DeviceOrientation skip this entire directory of tests. Those that do implement it should pass all tests. So an expected result which represents failure is probably not appropriate.

Perhaps you could move the test to a more appropriate location?

[Yongjun Zhang]

(In reply to comment #10)
> I'm not sure why you've added this test to fast/dom/DeviceOrientation. This LayoutTest directory is to test the implementation of the W3C DeviceOrientationEvent (http://dev.w3.org/geo/api/spec-source-orientation.html, https://bugs.webkit.org/show_bug.cgi?id=30335).
>
> Your change seems to be concerned with the OrientationEvent, which is completely separate.
>

Thanks for the comments, there are indeed different events.  I will move this test to a different location.
 
> Furthermore, as you point out, ports that don't implement DeviceOrientation skip this entire directory of tests. Those that do implement it should pass all tests. So an expected result which represents failure is probably not appropriate.
> 

I can add it to skipped tests for each port to get rid of the expected failure result.

Thanks.

[Steve Block]

> Thanks for the comments, there are indeed different events.  I will move this
> test to a different location.
Please move this test, or let me know where it should be moved to, else I think we should remove it.

[Laszlo Gombos]

Perhaps LayoutTests/fast/dom/Orientation ?

[Chang Shu]

Created attachment 75142 [details]
fix patch

move test case

[Laszlo Gombos]

Comment on attachment 75142 [details]
fix patch

LGTM, r+. Thanks.

[WebKit Commit Bot]

Comment on attachment 75142 [details]
fix patch

Rejecting patch 75142 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sl', 'build', '--no-clean', '--no-update', '--build-style=both']" exit_code: 1
ERROR: Working directory has local commits, pass --force-clean to continue.

Full output: http://queues.webkit.org/results/6512003

[Chang Shu]

Comment on attachment 75142 [details]
fix patch

cleanup flags after manual commit
r73019
http://trac.webkit.org/changeset/73019

[Steve Block]

Thanks!