[webkit-reviews] review denied: [Bug 91008] Web Inspector: Override the DeviceOrientation : [Attachment 151944] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jul 12 07:31:32 PDT 2012


Rob Buis <rwlbuis at gmail.com> has denied Konrad Piascik <kpiascik at rim.com>'s
request for review:
Bug 91008: Web Inspector: Override the DeviceOrientation
https://bugs.webkit.org/show_bug.cgi?id=91008

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

------- Additional Comments from Rob Buis <rwlbuis at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=151944&action=review


Looks good, some stuff to fix.

> Source/WebCore/inspector/InspectorInstrumentation.h:262
> +    static DeviceOrientation* overridenDeviceOrientation(Page*,
DeviceOrientation*);

Have you considered wrapping the whole function in #if ENABLE(INSPECTOR)?

> Source/WebCore/inspector/InspectorInstrumentation.h:1369
> +inline DeviceOrientation*
InspectorInstrumentation::overridenDeviceOrientation(Page* page,
DeviceOrientation* deviceOrientation)

You probably mean overrideDeviceOrientation, or overriddenDeviceOrientation?

> LayoutTests/ChangeLog:10
> +	   * inspector/device-orientation-success.html: Added.

No expected result?

> LayoutTests/inspector/device-orientation-success.html:26
> +	   function callbackComplete()

Indenting!


More information about the webkit-reviews mailing list