[webkit-reviews] review denied: [Bug 39210] Provide bindings for DeviceOrientation : [Attachment 56492] Patch

[bugzilla-daemon at webkit.org]

Jeremy Orlow <jorlow at chromium.org> has denied Steve Block
<steveblock at google.com>'s request for review:
Bug 39210: Provide bindings for DeviceOrientation
https://bugs.webkit.org/show_bug.cgi?id=39210

Attachment 56492: Patch
https://bugs.webkit.org/attachment.cgi?id=56492&action=review

------- Additional Comments from Jeremy Orlow <jorlow at chromium.org>
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?