Bug 92942

Summary: Add DeviceProximityEvent interface
Product: WebKit Reporter: Kihong Kwon <kihong.kwon>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Enhancement CC: abarth, anssi.kostiainen, arvind.tech225, dglazkov, gyuyoung.kim, haraken, morrita, ojan, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://www.w3.org/TR/proximity/#idl-def-DeviceProximityEvent
Bug Depends on:    
Bug Blocks: 92837    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
haraken: review+, haraken: commit-queue-
patch for landing.
none
Archive of layout-test-results from gce-cr-linux-01
none
patch for landing. none

Description Kihong Kwon 2012-08-01 22:16:01 PDT
Add DeviceProximityEvent interface.
And add onwebkitdeviceproximity event handler to the DOMWindow.
Comment 1 Kihong Kwon 2012-08-01 23:31:12 PDT
Created attachment 155981 [details]
Patch
Comment 2 Kihong Kwon 2012-08-01 23:55:18 PDT
Created attachment 155985 [details]
Patch
Comment 3 Adam Barth 2012-08-02 08:05:45 PDT
Looks like the spec is here:
http://www.w3.org/TR/proximity/#idl-def-DeviceProximityEvent
Comment 4 Kentaro Hara 2012-08-02 19:11:02 PDT
Comment on attachment 155985 [details]
Patch

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

WebCore implementation looks good. Marking r- due to insufficient tests.

> Source/WebCore/ChangeLog:9
> +        Add DeviceProximityEvent interface of Proximity Events.
> +        And add onwebkitdeviceproximity event handler to the DOMWindow.

Please add the spec link to the ChangeLog.

> Source/WebCore/Modules/proximity/DeviceProximityEvent.h:45
> +        return adoptRef(new DeviceProximityEvent);

Nit: new DeviceProximityEvent()

> LayoutTests/ChangeLog:8
> +        Add a test case to create DeviceProximityEvent and to check onwebkitdeviceproximity.

- Let's add test cases for constructor. Look at tests under fast/events/constructors/. Let's add fast/events/constructors/device-proximity-event.html

- Can you test if the DeviceProximityEvent bubbles up correctly?
Comment 5 Kihong Kwon 2012-08-06 01:27:41 PDT
Created attachment 156618 [details]
Patch
Comment 6 Kihong Kwon 2012-08-06 01:30:06 PDT
Comment on attachment 155985 [details]
Patch

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

>> Source/WebCore/Modules/proximity/DeviceProximityEvent.h:45
>> +        return adoptRef(new DeviceProximityEvent);
> 
> Nit: new DeviceProximityEvent()

OK.

>> LayoutTests/ChangeLog:8
>> +        Add a test case to create DeviceProximityEvent and to check onwebkitdeviceproximity.
> 
> - Let's add test cases for constructor. Look at tests under fast/events/constructors/. Let's add fast/events/constructors/device-proximity-event.html
> 
> - Can you test if the DeviceProximityEvent bubbles up correctly?

OK, I added a test case for constructor.
Comment 7 Kentaro Hara 2012-08-06 01:41:20 PDT
Comment on attachment 156618 [details]
Patch

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

> LayoutTests/fast/dom/Proximity/check-event-deviceproximity.html:9
> +<script src="script-tests/check-event-deviceproximity.js"></script>

Nit: You can directly write JavaScript here.

> LayoutTests/fast/dom/Proximity/create-event-deviceproximity.html:9
> +<script src="script-tests/create-event-deviceproximity.js"></script>

Ditto.

> LayoutTests/fast/events/constructors/device-proximity-event-constructor.html:14
> +shouldBe("new DeviceProximityEvent('eventType').bubbles", "false");

Isn't this true?
Comment 8 Kihong Kwon 2012-08-06 02:28:36 PDT
Created attachment 156632 [details]
Patch
Comment 9 Kihong Kwon 2012-08-06 02:35:48 PDT
Comment on attachment 156618 [details]
Patch

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

>> LayoutTests/fast/dom/Proximity/check-event-deviceproximity.html:9
>> +<script src="script-tests/check-event-deviceproximity.js"></script>
> 
> Nit: You can directly write JavaScript here.

Changed it.

>> LayoutTests/fast/dom/Proximity/create-event-deviceproximity.html:9
>> +<script src="script-tests/create-event-deviceproximity.js"></script>
> 
> Ditto.

ditto.

>> LayoutTests/fast/events/constructors/device-proximity-event-constructor.html:14
>> +shouldBe("new DeviceProximityEvent('eventType').bubbles", "false");
> 
> Isn't this true?

You are right.
It's fixed.
Comment 10 Kentaro Hara 2012-08-06 02:36:11 PDT
Comment on attachment 156632 [details]
Patch

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

Looks OK.

Sorry for the iterative comments. A couple of nits in your test.

> LayoutTests/fast/dom/Proximity/create-event-deviceproximity.html:14
> +    document.getElementById('result').innerHTML = "PASS";

Nit: you could use testPassed()

> LayoutTests/fast/dom/Proximity/create-event-deviceproximity.html:24
> +    document.getElementById('result').innerHTML = "FAIL... deviceproximity event doesn't appear to be enabled or implemented.";

Nit: you could use testFailed()
Comment 11 Kihong Kwon 2012-08-06 02:48:02 PDT
Comment on attachment 156632 [details]
Patch

No problem. Thank you :)
Comment 12 Kihong Kwon 2012-08-06 02:48:52 PDT
Created attachment 156640 [details]
patch for landing.
Comment 13 WebKit Review Bot 2012-08-06 03:42:08 PDT
Comment on attachment 156632 [details]
Patch

Attachment 156632 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13447242

New failing tests:
fast/events/constructors/device-proximity-event-constructor.html
Comment 14 WebKit Review Bot 2012-08-06 03:42:14 PDT
Created attachment 156648 [details]
Archive of layout-test-results from gce-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-01  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment 15 Kihong Kwon 2012-08-06 04:10:19 PDT
Created attachment 156652 [details]
patch for landing.
Comment 16 WebKit Review Bot 2012-08-06 06:04:21 PDT
Comment on attachment 156652 [details]
patch for landing.

Clearing flags on attachment: 156652

Committed r124759: <http://trac.webkit.org/changeset/124759>
Comment 17 WebKit Review Bot 2012-08-06 06:04:28 PDT
All reviewed patches have been landed.  Closing bug.
Comment 18 Lucas Forschler 2019-02-06 09:18:39 PST
Mass move bugs into the DOM component.