The default bug view has changed. See this FAQ.

Update WebTelephony impl to use the new DOMEventTargetHelper API

RESOLVED FIXED in mozilla14

Status

()

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

People

(Reporter: smaug, Assigned: fabrice)

Tracking

unspecified
mozilla14
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

3.12 KB, patch
Ben Turner (not reading bugmail, use the needinfo flag!)
: review+
smaug
: review+
Details | Diff | Splinter Review
Comment hidden (empty)
Summary: Update B2G EventTarget objects to use changed DOMEventTargetHelper API → Update B2G EventTarget objects to use the new DOMEventTargetHelper API
smaug, which ones are you specifically talking about?
Ask fabrice. I'm not familiar with b2g code, but apparently there are some
classes which extend nsDOMEventTargetHelper.
(Assignee)

Comment 3

5 years ago
Created attachment 605466 [details] [diff] [review]
patch

This patch fixes the build issues, but I'm not sure *at all* it's doing the right thing.
Attachment #605466 - Flags: feedback?(bugs)
Comment on attachment 605466 [details] [diff] [review]
patch

>+++ b/dom/telephony/Telephony.h
>@@ -118,16 +118,12 @@ public:
>     return mRIL;
>   }
> 
>-  nsPIDOMWindow*
>-  Owner() const
>+  nsIScriptContext*
>+  ScriptContext()
>   {
>-    return mOwner;
>-  }
>-
>-  nsIScriptContext*
>-  ScriptContext() const
>-  {
>-    return mScriptContext;
>+    nsresult rv;
>+    nsIScriptContext* sc = GetContextForEventHandlers(&rv);
>+    return sc;
>   }
I don't know why you some times use GetContextForEventHandlers, and yet have
this helper method.


> 
>   nsRefPtr<TelephonyCall> call = new TelephonyCall();
> 
>-  call->mOwner = aTelephony->Owner();
>-  call->mScriptContext = aTelephony->ScriptContext();
>   call->mTelephony = aTelephony;
>   call->mNumber = aNumber;
You don't bind the object to owner?
Attachment #605466 - Flags: feedback?(bugs) → feedback+
(Assignee)

Comment 5

5 years ago
Created attachment 605475 [details] [diff] [review]
patch v2

updated patch, thanks Olli for the quick review on the first one!
Assignee: nobody → fabrice
Attachment #605466 - Attachment is obsolete: true
Attachment #605475 - Flags: review?(bugs)
Attachment #605475 - Flags: review?(bent.mozilla)
Attachment #605475 - Flags: review?(bugs) → review+
Blocks: 674726
Component: General → DOM: Device Interfaces
Product: Boot2Gecko → Core
QA Contact: general → device-interfaces
Summary: Update B2G EventTarget objects to use the new DOMEventTargetHelper API → Update WebTelephony impl to use the new DOMEventTargetHelper API
(Assignee)

Updated

5 years ago
Blocks: 719491
Comment on attachment 605475 [details] [diff] [review]
patch v2

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

r=me with this addressed:

::: dom/telephony/Telephony.cpp
@@ +329,5 @@
>    }
>  
> +  nsresult rv;
> +  nsIScriptContext* sc = GetContextForEventHandlers(&rv);
> +  if (NS_SUCCEEDED(rv) && sc) {

Why not do NS_ENSURE_SUCCESS(rv, rv) here?

@@ +346,5 @@
>    JSObject* calls = mCallsArray;
>    if (!calls) {
> +    nsresult rv;
> +    nsIScriptContext* sc = GetContextForEventHandlers(&rv);
> +    if (NS_SUCCEEDED(rv) && sc) {

Here too.
Attachment #605475 - Flags: review?(bent.mozilla) → review+
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/mozilla-central/rev/c71845b3b2a6
(Assignee)

Updated

5 years ago
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
You need to log in before you can comment on or make changes to this bug.