superreview requested: [Bug 717462] WebTelephony: notify errors : [Attachment 616072] WebTelephony notify errors V2

[bugzill...@mozilla.org]

Philipp von Weitershausen [:philikon] <phi...@weitershausen.de> has asked
Jonas Sicking (:sicking) <jo...@sicking.cc> for superreview:
Bug 717462: WebTelephony: notify errors
https://bugzilla.mozilla.org/show_bug.cgi?id=717462

Attachment 616072: WebTelephony notify errors V2
https://bugzilla.mozilla.org/attachment.cgi?id=616072&action=edit


------- Additional Comments from Philipp von Weitershausen [:philikon]
<phi...@weitershausen.de>
Review of attachment 616072:
-----------------------------------------------------------------

Looks good to me! Paging sicking for super review on the interface changes.

::: dom/telephony/TelephonyCall.cpp
@@ +82,5 @@
> + nsRefPtr<CallEvent> event = CallEvent::Create(this);
> + NS_ASSERTION(event, "This should never fail!");
> +
> + if (NS_FAILED(event->Dispatch(ToIDOMEventTarget(),
> + NS_LITERAL_STRING("error")))) {

nit: align arguments

[bugzill...@mozilla.org]

Jonas Sicking (:sicking) <jo...@sicking.cc> has granted Philipp von
Weitershausen [:philikon] <phi...@weitershausen.de>'s request for superreview:

Bug 717462: WebTelephony: notify errors
https://bugzilla.mozilla.org/show_bug.cgi?id=717462

Attachment 616072: WebTelephony notify errors V2
https://bugzilla.mozilla.org/attachment.cgi?id=616072&action=edit

------- Additional Comments from Jonas Sicking (:sicking) <jo...@sicking.cc>

Review of attachment 616072:
-----------------------------------------------------------------

Let's make the nsIDOMTelephonyCall.error property an nsIDOMError (which
currently is just an object with a single string property on it, so you can
keep all the same logic and error values). That way we can add further
technical information in the future about the error if we want to. And checking
if something went wrong would still just be

if (call.error) { ... }