Bug 39210 - Provide bindings for DeviceOrientation
Summary: Provide bindings for DeviceOrientation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 39222
Blocks: 30335
  Show dependency treegraph
 
Reported: 2010-05-17 02:07 PDT by Steve Block
Modified: 2011-06-13 02:05 PDT (History)
11 users (show)

See Also:


Attachments
Patch (51.52 KB, patch)
2010-05-17 02:42 PDT, Steve Block
no flags Details | Formatted Diff | Diff
Patch (51.25 KB, patch)
2010-05-17 06:27 PDT, Steve Block
no flags Details | Formatted Diff | Diff
Patch (51.21 KB, patch)
2010-05-19 04:30 PDT, Steve Block
no flags Details | Formatted Diff | Diff
Patch (51.64 KB, patch)
2010-05-19 08:30 PDT, Steve Block
no flags Details | Formatted Diff | Diff
Patch (51.25 KB, patch)
2010-05-20 09:43 PDT, Steve Block
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Block 2010-05-17 02:07:16 PDT
Provide bindings for DeviceOrientation
Comment 1 Steve Block 2010-05-17 02:42:04 PDT
Created attachment 56228 [details]
Patch
Comment 2 Steve Block 2010-05-17 06:27:55 PDT
Created attachment 56237 [details]
Patch
Comment 3 WebKit Review Bot 2010-05-17 07:12:49 PDT
Attachment 56237 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/2294223
Comment 4 Steve Block 2010-05-17 09:32:25 PDT
The Chromium and qt build failures seem to be due to the V8 code generator failing to guard some code with ENABLE(DEVICE_ORIENTATION), which causes problems when the feature is disabled.

Have filed Bug 39222 to track this.
Comment 5 Hans Wennborg 2010-05-18 04:40:38 PDT
Adding myself to the CC list.
Comment 6 Steve Block 2010-05-19 04:30:01 PDT
Created attachment 56480 [details]
Patch
Comment 7 Steve Block 2010-05-19 04:31:39 PDT
Re-uploaded patch now that Bug 39222 is fixed.

Note that the large size of this patch is due mainly to boilerplate and LayoutTests.
Comment 8 Steve Block 2010-05-19 08:30:38 PDT
Created attachment 56492 [details]
Patch
Comment 9 Jeremy Orlow 2010-05-20 09:03:35 PDT
Comment on attachment 56492 [details]
Patch

Looking pretty good.


WebCore/Android.derived.v8bindings.mk:34
 +  FEATURE_DEFINES += V8_BINDING
Did you mean for this to be here?

WebCore/config.h:109
 +  #define ENABLE_DEVICE_ORIENTATION 1
Is this supposed to be here?

WebCore/dom/DeviceOrientationEvent.h:46
 +      double alpha() { return m_alpha; }
) const {

WebCore/dom/DeviceOrientationEvent.h:47
 +      double beta() { return m_beta; }
ditto

WebCore/dom/DeviceOrientationEvent.h:48
 +      double gamma() { return m_gamma; }
ditto

LayoutTests/platform/gtk/Skipped:1182
 +  fast/dom/DeviceOrientation/window-property.html
Why not the whole directory?

LayoutTests/platform/chromium/test_expectations.txt:2843
 +  DEFER SKIP : fast/dom/DeviceOrientation = TEXT
What's defer mean?

I made a bug for this.  s/DEFER/BUG44654/

WebKitLibraries/win/tools/vsprops/FeatureDefinesCairo.vsprops:57
 +  		PerformEnvironmentSet="true"
Is this supposed to be true?

WebKitLibraries/win/tools/vsprops/FeatureDefines.vsprops:57
 +  		PerformEnvironmentSet="true"
Is this supposed to be true?

LayoutTests/fast/dom/DeviceOrientation/script-tests/window-property.js:5
 +      for (var property in window) {
Why not just 'if ('ondeviceorientation' in window)' ?

LayoutTests/fast/dom/DeviceOrientation/script-tests/window-property.js:17
 +  window.jsTestIsAsync = false;
is this necessary?
Comment 10 Steve Block 2010-05-20 09:39:25 PDT
> WebCore/Android.derived.v8bindings.mk:34
>  +  FEATURE_DEFINES += V8_BINDING
> Did you mean for this to be here?
Yes - I split this out onto its own line because it's not really a feature like the others are.

> WebCore/config.h:109
>  +  #define ENABLE_DEVICE_ORIENTATION 1
It doesn't really matter - removed for now.

> WebCore/dom/DeviceOrientationEvent.h:46
>  +      double alpha() { return m_alpha; }
> ) const {
Done

> LayoutTests/platform/gtk/Skipped:1182
>  +  fast/dom/DeviceOrientation/window-property.html
> Why not the whole directory?
That's what the comments at the top of this file require - 'no grouping should occur. Every test for itself'

> LayoutTests/platform/chromium/test_expectations.txt:2843
>  +  DEFER SKIP : fast/dom/DeviceOrientation = TEXT
> What's defer mean?
From the comments as the top of the file - 'DEFER: For tests we'll fix in the future.'

> I made a bug for this.  s/DEFER/BUG44654/
Done

> WebKitLibraries/win/tools/vsprops/FeatureDefinesCairo.vsprops:57
>  +          PerformEnvironmentSet="true"
> Is this supposed to be true?
All of the flags in this file seem to set this.

> WebKitLibraries/win/tools/vsprops/FeatureDefines.vsprops:57
>  +          PerformEnvironmentSet="true"
> Is this supposed to be true?
Ditto

> LayoutTests/fast/dom/DeviceOrientation/script-tests/window-property.js:5
>  +      for (var property in window) {
> Why not just 'if ('ondeviceorientation' in window)' ?
We already test this on line 14. The difference is that properties which are marked 'DontEnum' will pass 'if (foo in bar)' but won't be enumerated with 'for (var x in bar)'. This test is copied from fast/dom/Geolocation/resources/enabled.js

> LayoutTests/fast/dom/DeviceOrientation/script-tests/window-property.js:17
>  +  window.jsTestIsAsync = false;
> is this necessary?
No, fixed.
Comment 11 Steve Block 2010-05-20 09:43:26 PDT
Created attachment 56602 [details]
Patch
Comment 12 Jeremy Orlow 2010-05-20 10:30:21 PDT
Comment on attachment 56602 [details]
Patch

r=me
Comment 13 Steve Block 2010-05-20 10:37:51 PDT
Comment on attachment 56602 [details]
Patch

Clearing flags on attachment: 56602

Committed r59847: <http://trac.webkit.org/changeset/59847>
Comment 14 Steve Block 2010-05-20 10:38:05 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 Olli Pettay (:smaug) 2011-06-10 15:17:06 PDT
Why did you end up adding non-standard ondeviceorientation property
to window object? (and even without webkit prefix)
Comment 16 Andrei Popescu 2011-06-13 02:05:25 PDT
(In reply to comment #15)
> Why did you end up adding non-standard ondeviceorientation property
> to window object? (and even without webkit prefix)

Hi Olli

Device Orientation is a specification published by the Geolocation WG at the W3C:

http://dev.w3.org/geo/api/spec-source-orientation.html

We've started the process to publish the FPWD last week. Doug Turner is Mozilla's representative in the Geolocation WG.

Thanks,
Andrei