Last Comment Bug 751663 - Implement new device proximity like event that fires only when the screen is close to the user's face.
: Implement new device proximity like event that fires only when the screen is ...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla15
Assigned To: Doug Turner (:dougt)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-03 11:55 PDT by Doug Turner (:dougt)
Modified: 2012-05-17 20:34 PDT (History)
6 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch v.1 (27.69 KB, patch)
2012-05-15 14:15 PDT, Doug Turner (:dougt)
bugs: review+
Details | Diff | Splinter Review

Description Doug Turner (:dougt) 2012-05-03 11:55:08 PDT
as a follow up to 738131 -- 

Me:

> I'd be happy with some specialized event - a separate event that gets fired
> only when the value changes.

Jonas: 

> That would be even better! I guess we can do that as a followup yes
Comment 1 Doug Turner (:dougt) 2012-05-10 14:21:40 PDT
The details are being spec'ed out in the dap wg.  more to follow.
Comment 2 Doug Turner (:dougt) 2012-05-15 14:15:40 PDT
Created attachment 624185 [details] [diff] [review]
patch v.1
Comment 4 Olli Pettay [:smaug] 2012-05-15 14:23:42 PDT
Comment on attachment 624185 [details] [diff] [review]
patch v.1


>+// UserProximityEvent
>+e = new UserProximityEvent("hello", {near: false});
>+ok(e.type, "hello", "Wrong event type!");
>+ok(!e.isTrusted, "Event should not be trusted");
>+is(e.near, false, "near should be false");

Please test near: true case, since false is the default value.



>-[scriptable, uuid(8fc58f56-f769-4368-a098-edd08550cf1a)]
>+[scriptable, uuid(f51bfe5f-cda7-4c9b-96b3-ec37817f91b8)]
> interface nsIDOMMozURLProperty : nsISupports
> {
Unrelated change
> nsDeviceSensors::nsDeviceSensors()
> {
>+  mIsUserProximityNear = true;
I would have assumed the initial value was false.
>+nsDeviceSensors::FireDOMUserProximityEvent(nsIDOMEventTarget *aTarget, bool aNear)
>+{
>+  nsCOMPtr<nsIDOMEvent> event;
>+  NS_NewDOMUserProximityEvent(getter_AddRefs(event), nsnull, nsnull);
>+  nsCOMPtr<nsIDOMUserProximityEvent> pe = do_QueryInterface(event);
>+
>+  pe->InitUserProximityEvent(NS_LITERAL_STRING("userproximity"),
>+			     true,
>+			     false,
>+			     aNear);

You have tabs in this code. Please use spaces.
Check also rest of the patch.

> 
>+  void FireDOMUserProximityEvent(nsIDOMEventTarget *aTarget,
>+				 bool aNear);
At least here as well
Comment 5 Justin Dolske [:Dolske] 2012-05-15 23:15:57 PDT
design guidelines: http://www.youtube.com/watch?v=iZhEcRrMA-M
Comment 6 Anssi Kostiainen 2012-05-16 01:00:29 PDT
(In reply to Doug Turner (:dougt) from comment #3)
> example page: http://dl.dropbox.com/u/8727858/mozilla/proximity/user.html
> 
> spec draft:
> http://dvcs.w3.org/hg/dap/raw-file/tip/proximity/userproximity.html

To clarify: the W3C Device APIs working group has not yet reached an agreement on the spec design. There's an alternative design on the table. See the proximitystate attribute section below:

  http://dvcs.w3.org/hg/dap/raw-file/tip/proximity/Overview.html#the-proximitystate-attribute

The above proposal addresses the following use case that is presumably not addressed by what is being implemented in this patch:

  http://lists.w3.org/Archives/Public/public-device-apis/2012May/0103.html

Given the prototype, you should experiment whether the concern raised above is valid, and provide feedback to the working group so that it can make an educated choice on the design. From an implementor's PoV it may make sense to re-use as much of the Device Proximity patch (https://bugzil.la/738131) as possible, but a good design should consider users and use cases over authors over implementors, IMHO.
Comment 7 Doug Turner (:dougt) 2012-05-16 10:21:02 PDT
Anssi, thanks for your comments. 

> To clarify: the W3C Device APIs working group has not yet reached an agreement on the spec design.

We typically do not wait for the w3c dap to reach agreement. 

> Given the prototype, you should experiment whether the concern raised above is valid, and provide feedback to the working group so that it can make an educated choice on the design.

Yup.  That is why we implement early.

> From an implementor's PoV it may make sense to re-use as much of the Device Proximity patch (https://bugzil.la/738131) as possible, but a good design should consider users and use cases over authors over implementors, IMHO.

That is what we did.  


This bug is for the firefox specific implementation.  Feel free to take these concerns to the w3c dap mailing list.

Doug
Comment 9 Ed Morley [:emorley] 2012-05-17 03:19:19 PDT
https://hg.mozilla.org/mozilla-central/rev/6d2f48e45174
Comment 10 :Ms2ger (⌚ UTC+1/+2) 2012-05-17 03:51:44 PDT
Comment on attachment 624185 [details] [diff] [review]
patch v.1

Review of attachment 624185 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/system/nsDeviceSensors.cpp
@@ +301,5 @@
> +			     false,
> +			     aNear);
> +
> +  nsCOMPtr<nsIPrivateDOMEvent> privateEvent = do_QueryInterface(event);
> +  privateEvent = do_QueryInterface(event);

Eh.
Comment 11 Olli Pettay [:smaug] 2012-05-17 04:02:29 PDT
Ugh, I should have noticed that.
Comment 12 Doug Turner (:dougt) 2012-05-17 08:20:40 PDT
i shouldn't have written it.

https://hg.mozilla.org/integration/mozilla-inbound/rev/73e3c505fb1a
Comment 13 Ryan VanderMeulen [:RyanVM] 2012-05-17 20:34:42 PDT
https://hg.mozilla.org/mozilla-central/rev/73e3c505fb1a

Note You need to log in before you can comment on or make changes to this bug.