Omitting data parameter in mozAlarms.add() method results in NS_ERROR_UNEXPECTED error

RESOLVED FIXED in mozilla36

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: salva, Assigned: seanlin)

Tracking

({regression})

unspecified
mozilla36
ARM
Gonk (Firefox OS)
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Created attachment 8513405 [details]
Test application to show the NS_ERROR_UNEXPECTED error when `data` parameter is not provided

According to documentation [1], parameter `data` of `navigator.mozAlarms.add()` method is optional but omitting it results in NS_ERROR_UNEXPECTED error.

Please, find an attached demo app which demonstrates this issue. Observe the console output when tapping on the first button.

[1] https://developer.mozilla.org/en-US/docs/Web/API/MozAlarmsManager.add
Component: DOM → DOM: Device Interfaces
This is a regression from bug 1015540 and bug 1037079.

The problem is that JSON.stringify(undefined) returns undefined, so JSON.parse(JSON.stringify(undefined)) throws.

We should skip the sandbox stuff if aData is undefined.... And clearly we need better tests here.
Flags: needinfo?(selin)
Created attachment 8514042 [details] [diff] [review]
Patch v1

Fixing the issue and adding correspondent tests.
Assignee: nobody → selin
Flags: needinfo?(selin)
Attachment #8514042 - Flags: review?(bzbarsky)
Comment on attachment 8514042 [details] [diff] [review]
Patch v1

Thank you for writing the test.

>+    secondsLater.setSeconds(secondsLater.getSeconds() + 10);

Why 10, not 1 or 0.01?  There's no reason to wait for 10s here during a test run.

>+  SimpleTest.expectAssertions(0, 9);

Why are there assertions?  Because you reload the window?

I suggest avoiding the reloading business (which is liable to confuse the test harness badly) by running the parts of the test that depend on the permission bit in an iframe.  Add the permission, then load the iframe, which should then be able to do stuff with the alarms API.  That should work, no?

r=me with the tests fixed up
Attachment #8514042 - Flags: review?(bzbarsky) → review+
Created attachment 8516405 [details] [diff] [review]
Patch v2

Updating based on r+ comments.
Attachment #8514042 - Attachment is obsolete: true
Attachment #8516405 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/0ca912d0526c
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.