738465 - implement device light sensor

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Doug Turner (:dougt)]

We need to expose the light sensor to content.

Comment 1

[Doug Turner (:dougt)]

Attachment: patch v.1

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 609383 [details] [diff] [review]
patch v.1


>+NS_IMETHODIMP nsDOMDeviceLightEvent::InitDeviceLightEvent(const nsAString & aEventTypeArg,
>+                                                                      bool aCanBubbleArg,
>+                                                                      bool aCancelableArg,
>+                                                                      double aValue)
NS_IMETHODIMP should be in its own line here and elsewhere.
Align parameters.


>+nsresult NS_NewDOMDeviceLightEvent(nsIDOMEvent** aInstancePtrResult,
>+                                         nsPresContext* aPresContext,
>+                                         nsEvent *aEvent) 
>+{
>+  NS_ENSURE_ARG_POINTER(aInstancePtrResult);
>+
>+  nsDOMDeviceLightEvent* it = new nsDOMDeviceLightEvent(aPresContext, aEvent);
>+  if (nsnull == it) {
>+    return NS_ERROR_OUT_OF_MEMORY;
>+  }
No need for this null check.

>+  if (aEventType.LowerCaseEqualsLiteral("devicelightevent"))
>+    return NS_NewDOMDeviceLightEvent(aDOMEvent, aPresContext, nsnull);
If possible, don't add this, but implement event ctor.

>+      } else if (aType == NS_DEVICE_LIGHT) {
>+        nsPIDOMWindow* window = GetInnerWindowForTarget();
>+        if (window)
>+          window->DisableDeviceSensor(SENSOR_LIGHT);
if (expr) {
  stmt;
}

>+[scriptable, uuid(c6f68e93-9eaf-4bba-852b-c9c534a8d079)]
>+interface nsIDOMDeviceLightEvent : nsIDOMEvent
>+{
>+  void initDeviceLightEvent(in DOMString eventTypeArg,
>+			    in boolean canBubbleArg,
>+			    in boolean cancelableArg,
>+			    in double value);
Make init*Event [noscript] and implement event ctor using a dictionary

> void
>+nsDeviceSensors::FireDOMLightEvent(nsIDOMDocument *domdoc,
>+				  nsIDOMEventTarget *target,
>+				  double value)
align parameters. Parameter names should be aName



>+{
>+  nsCOMPtr<nsIDOMEvent> event;
>+  bool defaultActionEnabled = true;
>+  domdoc->CreateEvent(NS_LITERAL_STRING("DeviceLightEvent"), getter_AddRefs(event));
This could just call NS_NewDOMDeviceLightEvent

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

er, I was going to r-

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I'd really like to expose the current light value somewhere, as well as a switch to turn on/off the events. I think it's a lot easier for applications to flip a boolean property than to add/remove event listeners any time they want to sample the current background light.

Comment 5

[Mounir Lamouri (:mounir)]

(In reply to Jonas Sicking (:sicking) from comment #4)
> I'd really like to expose the current light value somewhere, as well as a
> switch to turn on/off the events. I think it's a lot easier for applications
> to flip a boolean property than to add/remove event listeners any time they
> want to sample the current background light.

+1. I think we should do something like the Battery API where you can do a direct access to the value. Otherwise, you are going to register for an event and I guess your implementation will fire an event immediately so the web page will know the current value. That looks wrong...

Comment 6

[Doug Turner (:dougt)]

jonas and mounir - exposing a sensor like that is a bad idea as I have mentioned before - it requires that you return undefined before it is called and you have no way of informing the implementation that the sensor can be disabled.  Since your approach is concerned with not just this api, but every device api, lets please take it to the mailing list. even if I am wrong (which I doubt :) ), that work can be done in a follow up.

Comment 7

[Mounir Lamouri (:mounir)]

I will let you reply to my message in dev-webapi but I don't see why you will have to return undefined unless there are some implementations limitations I'm not aware of.

Comment 8

[Doug Turner (:dougt)]

> I will let you reply to my message in dev-webapi but I don't see why you will have to return undefined unless there are some implementations limitations I'm not aware of.

I'll write something up to express why doing this is an extremely bad idea.

Comment 9

[Doug Turner (:dougt)]

Attachment: patch .2

Comment 10

[Doug Turner (:dougt)]

Jonas - responded there.  Per our conversation this afternoon, this is something that can be done as a follow up (if I am wrong).

Comment 11

[Doug Turner (:dougt)]

Attachment: rebased

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 612653 [details] [diff] [review]
rebased

Someone else needs to review hal/ and android parts.
And I wish Jonas could give feedback for the API.

Comment 13

[Mounir Lamouri (:mounir)]

Comment on attachment 612653 [details] [diff] [review]
rebased

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

r=me for hal changes. I can't review Android backend changes.

Comment 14

[Doug Turner (:dougt)]

Comment on attachment 612653 [details] [diff] [review]
rebased

blassey for the android change

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 612653 [details] [diff] [review]
rebased

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

We need to document what the unit of .value is. Also, hide the initDeviceLightEvent function from web pages since we have the ctor (as commented in the proximity bug). r=me with that.

Comment 16

[Doug Turner (:dougt)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bbdf0f8c06a8

Comment 17

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/bbdf0f8c06a8

Comment 18

[Florian Scholz (Open Web Docs)]

Mentioned on:
https://developer.mozilla.org/en-US/docs/DOM/DOM_event_reference
https://developer.mozilla.org/en-US/docs/Firefox_15_for_developers#DOM
New DOM event page:
https://developer.mozilla.org/en-US/docs/DOM/DeviceLightEvent