Closed
Bug 735305
Opened 13 years ago
Closed 13 years ago
Update WebTelephony impl to use the new DOMEventTargetHelper API
Categories
(Core :: DOM: Device Interfaces, defect)
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: smaug, Assigned: fabrice)
References
Details
Attachments
(1 file, 1 obsolete file)
|
(deleted),
patch
|
bent.mozilla
:
review+
smaug
:
review+
|
Details | Diff | Splinter Review |
No description provided.
|
Reporter | |
Updated•13 years ago
|
Summary: Update B2G EventTarget objects to use changed DOMEventTargetHelper API → Update B2G EventTarget objects to use the new DOMEventTargetHelper API
|
||
Comment 1•13 years ago
|
||
smaug, which ones are you specifically talking about?
|
|
Reporter | |
Comment 2•13 years ago
|
||
Ask fabrice. I'm not familiar with b2g code, but apparently there are some
classes which extend nsDOMEventTargetHelper.
|
Assignee | |
Comment 3•13 years ago
|
||
This patch fixes the build issues, but I'm not sure *at all* it's doing the right thing.
Attachment #605466 -
Flags: feedback?(bugs)
|
|
Reporter | |
Comment 4•13 years ago
|
||
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•13 years ago
|
||
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)
|
|
Reporter | |
Updated•13 years ago
|
Attachment #605475 -
Flags: review?(bugs) → review+
|
|
||
Updated•13 years ago
|
Blocks: webtelephony
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
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•13 years ago
|
||
|
|
Assignee | |
Updated•13 years ago
|
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
|
||
Updated•13 years ago
|
Target Milestone: --- → mozilla14
You need to log in
before you can comment on or make changes to this bug.
Description
•