Closed Bug 1023148 Opened 6 years ago Closed 6 years ago

Use WebIDL codegen to implement RIL & WiFi events

Categories

(Core :: DOM: Device Interfaces, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
2.0 S4 (20june)

People

(Reporter: vicamo, Assigned: vicamo)

References

Details

(Whiteboard: [ft:ril][p=1])

Attachments

(1 file, 1 obsolete file)

We have MozCellBroadcastEvent, MozSmsEvent, MozMmsEvent, CallEvent, MozVoicemailEvent, MozWifiConnectionInfoEvent, MozWifiP2pStatusChangeEvent, MozWifiStationInfoEvent, MozWifiStatusChangeEvent.
Blocks: 859764
Attached patch patch (obsolete) — Splinter Review
Full try: https://tbpl.mozilla.org/?tree=Try&rev=edbcb86d2a9f
Assignee: nobody → vyang
Attachment #8437566 - Flags: review?(bugs)
Whiteboard: [ft:ril][p=1]
Target Milestone: --- → 2.0 S4 (20june)
Comment on attachment 8437566 [details] [diff] [review]
patch

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

::: dom/webidl/MozWifiStatusChangeEvent.webidl
@@ +3,5 @@
>   * License, v. 2.0. If a copy of the MPL was not distributed with this file,
>   * You can obtain one at http://mozilla.org/MPL/2.0/.
>   */
>  
>  [Constructor(DOMString type, optional MozWifiStatusChangeEventInit eventInitDict), HeaderFile="GeneratedEventClasses.h"]

nit: will remove |HeaderFile="GeneratedEventClasses.h"| in all converted events in next revision.
Comment on attachment 8437566 [details] [diff] [review]
patch

> 
>-  nsRefPtr<CallEvent> event = CallEvent::Create(this, aType, aCall, false, false);
>+  CallEventInit init;
>+  init.mBubbles = false;
>+  init.mCancelable = false;
>+  if (aCall) {
>+    init.mCall = aCall;
>+  }
Why you need the null check here?


>+  nsRefPtr<MozVoicemailEvent> event =
>+      MozVoicemailEvent::Constructor(this, NS_LITERAL_STRING("statuschanged"), init);
2 spaces for indentation, please
Attachment #8437566 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #3)
> Comment on attachment 8437566 [details] [diff] [review]
> patch
> 
> >-  nsRefPtr<CallEvent> event = CallEvent::Create(this, aType, aCall, false, false);
> >+  CallEventInit init;
> >+  init.mBubbles = false;
> >+  init.mCancelable = false;
> >+  if (aCall) {
> >+    init.mCall = aCall;
> >+  }
> Why you need the null check here?

Thank you for the quick feedback. I don't really need it.
Attached patch patch : v2Splinter Review
Address review comment 2 and comment 3.  Also found another indentation error in MozCellBroadcastEvent.webidl.  Waiting for try results to land.
Attachment #8437566 - Attachment is obsolete: true
Attachment #8437614 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/1882ccda42c0
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.