implement device light sensor

RESOLVED FIXED in mozilla15

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
5 years ago
3 months ago

People

(Reporter: dougt, Assigned: dougt)

Tracking

(Depends on: 2 bugs, Blocks: 1 bug, {dev-doc-complete})

Trunk
mozilla15
dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
We need to expose the light sensor to content.
OS: Mac OS X → All
Hardware: x86 → All
Version: unspecified → Trunk
(Assignee)

Comment 1

5 years ago
Created attachment 609383 [details] [diff] [review]
patch v.1
Attachment #609383 - Flags: review?(bugs)

Comment 2

5 years ago
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
Attachment #609383 - Flags: review?(bugs) → review+

Updated

5 years ago
Attachment #609383 - Flags: review+ → review-

Comment 3

5 years ago
er, I was going to r-
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.
(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...
(Assignee)

Comment 6

5 years ago
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.
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.
(Assignee)

Comment 8

5 years ago
> 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.
(Assignee)

Comment 9

5 years ago
Created attachment 609999 [details] [diff] [review]
patch .2
Attachment #609383 - Attachment is obsolete: true
(Assignee)

Comment 10

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

Comment 11

5 years ago
Created attachment 612653 [details] [diff] [review]
rebased
Attachment #609999 - Attachment is obsolete: true
Attachment #612653 - Flags: review?(bugs)
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.
Attachment #612653 - Flags: review?(bugs)
Attachment #612653 - Flags: review+
Attachment #612653 - Flags: feedback?(jonas)
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.
Attachment #612653 - Flags: review+
(Assignee)

Comment 14

5 years ago
Comment on attachment 612653 [details] [diff] [review]
rebased

blassey for the android change
Attachment #612653 - Flags: review?(blassey.bugs)
Attachment #612653 - Flags: review?(blassey.bugs) → review+
(Assignee)

Updated

5 years ago
Attachment #612653 - Flags: feedback?(jonas) → review?(jonas)
Keywords: dev-doc-needed
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.
Attachment #612653 - Flags: review?(jonas) → review+
(Assignee)

Comment 16

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/bbdf0f8c06a8
https://hg.mozilla.org/mozilla-central/rev/bbdf0f8c06a8
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15

Updated

5 years ago
Depends on: 754199
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
Keywords: dev-doc-needed → dev-doc-complete
Blocks: 842952
Depends on: 1359124
You need to log in before you can comment on or make changes to this bug.