Closed
Bug 931307
Opened 11 years ago
Closed 11 years ago
"Assertion failure: !aTitle.IsEmpty()" with DOM Notification
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla28
People
(Reporter: jruderman, Assigned: baku)
References
Details
(Keywords: assertion, testcase, Whiteboard: [systemsfe][qa-])
Attachments
(4 files, 1 obsolete file)
Assertion failure: !aTitle.IsEmpty(), at dom/src/notification/Notification.cpp:63
Reporter | ||
Comment 1•11 years ago
|
||
Updated•11 years ago
|
Whiteboard: [systemsfe]
Assignee | ||
Comment 2•11 years ago
|
||
Following the spec, title can be an empty string.
Attachment #823365 -
Flags: review?(wchen)
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → amarchesini
Assignee | ||
Comment 3•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
OS: Mac OS X → All
Hardware: x86_64 → All
Comment 4•11 years ago
|
||
Comment on attachment 823365 [details] [diff] [review]
notification.patch
Review of attachment 823365 [details] [diff] [review]:
-----------------------------------------------------------------
Hijacking this review since the bug was caused by my code.
::: dom/tests/mochitest/notification/mochitest.ini
@@ +4,5 @@
> NotificationTest.js
>
> [test_notification_basics.html]
> [test_notification_storage.html]
> +[test_test_bug931307.html]
This should be:
[test_bug931307.html]
::: dom/tests/mochitest/notification/test_bug931307.html
@@ +13,5 @@
> +<script type="application/javascript"><!--
> +
> +new Notification("");
> +Notification.get();
> +ok(true, "No crash!");
The crash was happening in NotificationStorageCallback::Handle, which happens asynchronously from this content script. To be on the safe side, let's only succeed in the promise callback. So perhaps something like:
SimpleTest.waitForExplicitFinish();
new Notification("");
var promise = Notification.get();
promise.then(
function onSuccess() {
ok(true, "No crash!");
SimpleTest.finish();
},
function onFailure() {
ok(false, "Should not get an error in promise callback");
}
);
Assignee | ||
Comment 5•11 years ago
|
||
Attachment #823365 -
Attachment is obsolete: true
Attachment #823365 -
Flags: review?(wchen)
Attachment #824387 -
Flags: review?(mhenretty)
Comment 6•11 years ago
|
||
Comment on attachment 824387 [details] [diff] [review]
notification.patch
Review of attachment 824387 [details] [diff] [review]:
-----------------------------------------------------------------
LGTM!
We need some tests up in gaia for this too, so I'll add that to this bug.
Attachment #824387 -
Flags: review?(mhenretty) → review+
Updated•11 years ago
|
Whiteboard: [systemsfe] → [systemsfe][keep open]
Comment 7•11 years ago
|
||
Once the Gecko patch lands, I'll re-kick off the travis test and flag Kyle for review.
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 8•11 years ago
|
||
Flags: in-testsuite+
Keywords: checkin-needed
Comment 9•11 years ago
|
||
Comment 10•11 years ago
|
||
Comment on attachment 824739 [details] [review]
Pull Request for Gaia Integration Test
Kyle, will you review this crash test up in gaia land?
Attachment #824739 -
Flags: review?(kyle)
Updated•11 years ago
|
Attachment #824739 -
Flags: review?(kyle) → review+
Comment 11•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Target Milestone: --- → mozilla28
Reporter | ||
Updated•11 years ago
|
Whiteboard: [systemsfe][keep open] → [systemsfe]
Updated•11 years ago
|
Whiteboard: [systemsfe] → [systemsfe][qa-]
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•