Closed
Bug 849739
Opened 12 years ago
Closed 12 years ago
B2G MMS: provide nsIDOMMobileMessageManager.getThreads()
Categories
(Core :: DOM: Device Interfaces, defect)
Tracking
()
People
(Reporter: airpingu, Assigned: vicamo)
References
Details
(Keywords: dev-doc-needed)
Attachments
(5 files, 26 obsolete files)
(deleted),
patch
|
vicamo
:
review+
vicamo
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
vicamo
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
vicamo
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
vicamo
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
vicamo
:
review+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #844431 +++ After bug 844431 is done, we can have a |nsIDOMMobileMessageManager| interface to add: nsIDOMDOMRequest getThreadList(); and then deprecate the following one in the |nsIDOMMobileMessageManager|: nsIDOMMozSmsRequest getThreadList(); This function needs to be compatible with both SMS and MMS.
Reporter | ||
Comment 1•12 years ago
|
||
Vicamo has a better road map in his mind. Let him take this. Btw, this one should be leo+ because it's blocking an MMS user story.
Assignee: nobody → vyang
Assignee | ||
Comment 3•12 years ago
|
||
Attachment #724527 -
Flags: feedback?(mounir)
Attachment #724527 -
Flags: feedback?(fbsc)
Assignee | ||
Comment 4•12 years ago
|
||
DOM & IPC done, just have to implement MobileMessageThread & fix backends.
Assignee | ||
Updated•12 years ago
|
Summary: B2G MMS: provide nsIDOMMobileMessageManager.getThreadList() → B2G MMS: provide nsIDOMMobileMessageManager.getThreads()
Updated•12 years ago
|
Attachment #724527 -
Flags: feedback?(mounir) → feedback?(jonas)
Assignee | ||
Comment 5•12 years ago
|
||
1) Rebase to bug 838467 and bug 844431, fix MobileMessageManager as well. 2) Remove nsIDOMMozMobileMessageThread::subject, not required. 3) Add nsIMobileMessageService::createThread for creating instances from js. 4) s/struct MobileMessageThreadData/struct ThreadData/ because it's already in dom::mozilla::mobilemessage namespace.
Attachment #724527 -
Attachment is obsolete: true
Attachment #724527 -
Flags: feedback?(jonas)
Attachment #724527 -
Flags: feedback?(fbsc)
Attachment #724992 -
Flags: feedback?(jonas)
Attachment #724992 -
Flags: feedback?(fbsc)
Assignee | ||
Comment 6•12 years ago
|
||
Assignee | ||
Comment 7•12 years ago
|
||
Assignee | ||
Comment 8•12 years ago
|
||
Assignee | ||
Comment 9•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Attachment #724528 -
Attachment is obsolete: true
Assignee | ||
Comment 11•12 years ago
|
||
Keep the same attribute type and naming as bug 850127.
Attachment #724992 -
Attachment is obsolete: true
Attachment #724992 -
Flags: feedback?(jonas)
Attachment #724992 -
Flags: feedback?(fbsc)
Attachment #725494 -
Flags: feedback?(jonas)
Attachment #725494 -
Flags: feedback?(fbsc)
Assignee | ||
Updated•12 years ago
|
Attachment #725494 -
Attachment description: Part 1/5: interface changes → Part 1/5: interface changes : v2
Assignee | ||
Comment 12•12 years ago
|
||
Keep the same attribute type and naming as bug 850127.
Attachment #724993 -
Attachment is obsolete: true
Assignee | ||
Comment 13•12 years ago
|
||
Extract common validations and document the test cases more.
Attachment #724997 -
Attachment is obsolete: true
Updated•12 years ago
|
Whiteboard: [target 3/22]
Reporter | ||
Updated•12 years ago
|
Keywords: dev-doc-needed
Assignee | ||
Comment 14•12 years ago
|
||
Can we mark it as leo+ now? It's also blocking 847744, which is already a leo+.
Updated•12 years ago
|
blocking-b2g: leo? → leo+
Assignee | ||
Comment 15•12 years ago
|
||
Rebase to m-c tip to include MMS changes
Attachment #725494 -
Attachment is obsolete: true
Attachment #725494 -
Flags: feedback?(jonas)
Attachment #725494 -
Flags: feedback?(fbsc)
Attachment #728106 -
Flags: superreview?(jonas)
Attachment #728106 -
Flags: review?(bent.mozilla)
Assignee | ||
Comment 16•12 years ago
|
||
1) Rebase 2) follow variable naming in bug 838467
Attachment #725496 -
Attachment is obsolete: true
Attachment #728107 -
Flags: review?(jonas)
Assignee | ||
Comment 17•12 years ago
|
||
1) follow review comments in bug 838476 2) move a few code into MobileMessageCursorParent::DoRequest as done in SmsRequestParent. 3) correct variable naming
Attachment #724995 -
Attachment is obsolete: true
Attachment #728109 -
Flags: review?(bent.mozilla)
Assignee | ||
Comment 18•12 years ago
|
||
Rebase only.
Attachment #724996 -
Attachment is obsolete: true
Attachment #728110 -
Flags: review?(anygregor)
Assignee | ||
Comment 19•12 years ago
|
||
Refactor. Much much cleaner and well documented.
Attachment #725497 -
Attachment is obsolete: true
Attachment #728111 -
Flags: review?(anygregor)
Updated•12 years ago
|
Attachment #728106 -
Flags: superreview?(jonas) → superreview?(mounir)
Updated•12 years ago
|
Attachment #728107 -
Flags: review?(jonas) → review?(mounir)
Comment 21•12 years ago
|
||
Comment on attachment 728106 [details] [diff] [review] Part 1/5: interface changes : v3 Review of attachment 728106 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/mobilemessage/interfaces/nsIDOMMobileMessageManager.idl @@ +42,5 @@ > nsIDOMDOMCursor getMessages(in nsIDOMMozSmsFilter filter, in boolean reverse); > > nsIDOMDOMRequest markMessageRead(in long id, in boolean aValue); > > + nsIDOMDOMCursor getThreads(); Could you add a comment saying that the DOMCursor will iterate trough MobileMessageThread objects? ::: dom/mobilemessage/interfaces/nsIDOMMozMobileMessageThread.idl @@ +7,5 @@ > +[scriptable, builtinclass, uuid(ebc1db1a-07f9-4d79-b487-150298900ccc)] > +interface nsIDOMMozMobileMessageThread : nsISupports > +{ > + readonly attribute unsigned long long id; > + readonly attribute DOMString body; What's the body? Does it represent the last message's body? ::: dom/mobilemessage/interfaces/nsIDOMSmsManager.idl @@ +30,5 @@ > nsIDOMDOMCursor getMessages(in nsIDOMMozSmsFilter filter, in boolean reverse); > > nsIDOMMozSmsRequest markMessageRead(in long id, in boolean aValue); > > + nsIDOMDOMCursor getThreads(); ditto
Attachment #728106 -
Flags: superreview?(mounir) → superreview+
Comment 22•12 years ago
|
||
Comment on attachment 728107 [details] [diff] [review] Part 2/5: DOM : v3 Review of attachment 728107 [details] [diff] [review]: ----------------------------------------------------------------- r=me but I would like Blake to look at MobileMessageThread::Create() (MobileMessageThread.cpp). ::: dom/mobilemessage/src/MobileMessageThread.cpp @@ +46,5 @@ > + data.unreadCount() = aUnreadCount; > + > + // Participants. > + { > + if (aParticipants == JSVAL_NULL) { Shouldn't that be: if (aParticipants.isObject()) { and then the NULL case will anyway be handled by toObjectOrNull() call? @@ +90,5 @@ > + data.timestamp() = static_cast<uint64_t>(number); > + } > + > + nsCOMPtr<nsIDOMMozMobileMessageThread> thread = new MobileMessageThread(data); > + thread.swap(*aThread); You should actually use .forget(). ::: dom/mobilemessage/src/MobileMessageThread.h @@ +16,5 @@ > +namespace dom { > + > +class MobileMessageThread MOZ_FINAL : public nsIDOMMozMobileMessageThread > +{ > + typedef mobilemessage::ThreadData ThreadData; Could you add 'private:' before. Explicit is better ;) @@ +34,5 @@ > + MobileMessageThread(const uint64_t aId, > + const nsString& aBody, > + const uint64_t aUnreadCount, > + const nsTArray<nsString>& aParticipants, > + const uint64_t aTimestamp); nit: I would put the ctor first. Also, should you MOZ_DELETE the default ctor or is it fine to use it? ::: dom/tests/mochitest/general/test_interfaces.html @@ +543,5 @@ > "Gamepad", > "GamepadEvent", > "GamepadButtonEvent", > + "GamepadAxisMoveEvent", > + "MozMobileMessageThread" nit: add a ',' after the interface name so the next person doesn't have to do it when adding something here.
Attachment #728107 -
Flags: review?(mrbkap)
Attachment #728107 -
Flags: review?(mounir)
Attachment #728107 -
Flags: review+
Assignee | ||
Comment 23•12 years ago
|
||
(In reply to Mounir Lamouri (:mounir) from comment #21) > Part 1/5: interface changes : v3 > Review of attachment 728106 [details] [diff] [review]: > ----------------------------------------------------------------- > ::: dom/mobilemessage/interfaces/nsIDOMMozMobileMessageThread.idl > @@ +7,5 @@ > > +[scriptable, builtinclass, uuid(ebc1db1a-07f9-4d79-b487-150298900ccc)] > > +interface nsIDOMMozMobileMessageThread : nsISupports > > +{ > > + readonly attribute unsigned long long id; > > + readonly attribute DOMString body; > > What's the body? Does it represent the last message's body? Yes. Will have comments for all these new interface/attributes. Thank you :)
Assignee | ||
Comment 24•12 years ago
|
||
(In reply to Mounir Lamouri (:mounir) from comment #22) > Part 2/5: DOM : v3 > Review of attachment 728107 [details] [diff] [review]: > ----------------------------------------------------------------- > ::: dom/mobilemessage/src/MobileMessageThread.cpp > @@ +46,5 @@ > > + data.unreadCount() = aUnreadCount; > > + > > + // Participants. > > + { > > + if (aParticipants == JSVAL_NULL) { > > Shouldn't that be: > if (aParticipants.isObject()) { > and then the NULL case will anyway be handled by toObjectOrNull() call? All attributes in MobileMessageThread are read-only and the only caller of MobileMessageThread::Create() must be Gecko itself. But, sure, we can do some more the ensure it again here. > ::: dom/mobilemessage/src/MobileMessageThread.h > @@ +34,5 @@ > > + MobileMessageThread(const uint64_t aId, > > + const nsString& aBody, > > + const uint64_t aUnreadCount, > > + const nsTArray<nsString>& aParticipants, > > + const uint64_t aTimestamp); > > nit: I would put the ctor first. > > Also, should you MOZ_DELETE the default ctor or is it fine to use it? I was told not to do so when we have already a non-trivial ctor. That sounds more reasonable because the reason we want to MOZ_DELETE the default ctor is to prevent accidental use of automatically generated default ctor. Since compiler won't generate an additional trivial ctor at all in this case, the reason no longer exists and we don't have to add one additional line here. > ::: dom/tests/mochitest/general/test_interfaces.html > @@ +543,5 @@ > > "Gamepad", > > "GamepadEvent", > > "GamepadButtonEvent", > > + "GamepadAxisMoveEvent", > > + "MozMobileMessageThread" > > nit: add a ',' after the interface name so the next person doesn't have to > do it when adding something here. Glad to :)
Comment 25•12 years ago
|
||
Comment on attachment 728107 [details] [diff] [review] Part 2/5: DOM : v3 Review of attachment 728107 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/mobilemessage/src/MobileMessageThread.cpp @@ +51,5 @@ > + return NS_ERROR_INVALID_ARG; > + } > + > + JSObject* obj = aParticipants.toObjectOrNull(); > + if (!obj || !JS_IsArrayObject(aCx, obj)) { I'd write this as: JSObject* obj = &aParticipants.toObject(); if (!JS_IsArrayObject(... to be clear that obj can't be null. @@ +89,5 @@ > + } > + data.timestamp() = static_cast<uint64_t>(number); > + } > + > + nsCOMPtr<nsIDOMMozMobileMessageThread> thread = new MobileMessageThread(data); MobileMessageThread asserts that there is at least one participant, but that invariant isn't enforced in the above code. Should we throw if aParticipants is an empty array? @@ +148,5 @@ > +NS_IMETHODIMP > +MobileMessageThread::GetTimestamp(JSContext* aCx, > + JS::Value* aDate) > +{ > + *aDate = OBJECT_TO_JSVAL(JS_NewDateObjectMsec(aCx, mData.timestamp())); JS_NewDateObjectMsec can throw (return null). You should catch that and return a failure code in that case.
Attachment #728107 -
Flags: review?(mrbkap) → review+
Comment 26•12 years ago
|
||
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #24) > > ::: dom/mobilemessage/src/MobileMessageThread.h > > @@ +34,5 @@ > > > + MobileMessageThread(const uint64_t aId, > > > + const nsString& aBody, > > > + const uint64_t aUnreadCount, > > > + const nsTArray<nsString>& aParticipants, > > > + const uint64_t aTimestamp); > > > > nit: I would put the ctor first. > > > > Also, should you MOZ_DELETE the default ctor or is it fine to use it? > > I was told not to do so when we have already a non-trivial ctor. That sounds > more reasonable because the reason we want to MOZ_DELETE the default ctor is > to prevent accidental use of automatically generated default ctor. Since > compiler won't generate an additional trivial ctor at all in this case, the > reason no longer exists and we don't have to add one additional line here. You know what? I thought that the compiler was always creating the default ctor the same way there is always a copy ctor. I learned something today ;)
Assignee | ||
Comment 27•12 years ago
|
||
Address review comment 21.
Attachment #728106 -
Attachment is obsolete: true
Attachment #728106 -
Flags: review?(bent.mozilla)
Attachment #730132 -
Flags: superreview+
Attachment #730132 -
Flags: review?(bent.mozilla)
Assignee | ||
Comment 28•12 years ago
|
||
Address review comment 22 and comment 25.
Attachment #728107 -
Attachment is obsolete: true
Attachment #730133 -
Flags: review+
Assignee | ||
Comment 29•12 years ago
|
||
Address review comments in bug 838467.
Attachment #728109 -
Attachment is obsolete: true
Attachment #728109 -
Flags: review?(bent.mozilla)
Attachment #730135 -
Flags: review?(bent.mozilla)
Assignee | ||
Comment 30•12 years ago
|
||
inline createThreadFromRecord() because it's only called in GetThreadsCursor.
Attachment #728110 -
Attachment is obsolete: true
Attachment #728110 -
Flags: review?(anygregor)
Attachment #730137 -
Flags: review?(anygregor)
Assignee | ||
Comment 31•12 years ago
|
||
The last patch doesn't address bug 838467 comment 57. Fixed.
Attachment #730135 -
Attachment is obsolete: true
Attachment #730135 -
Flags: review?(bent.mozilla)
Attachment #730159 -
Flags: review?(bent.mozilla)
Updated•12 years ago
|
Attachment #730132 -
Flags: review?(bent.mozilla) → review+
Comment on attachment 730159 [details] [diff] [review] Part 3/5: IPC : v4 Review of attachment 730159 [details] [diff] [review]: ----------------------------------------------------------------- r=me with these changes. ::: dom/mobilemessage/src/ipc/SmsParent.cpp @@ +397,5 @@ > + getter_AddRefs(mContinueCallback)); > + } > + > + if (NS_FAILED(rv)) { > + rv = NotifyCursorError(nsIMobileMessageCallback::INTERNAL_ERROR); Nit: This could just be |return NotifyCursorError(...);| and then below you could always return true. @@ +422,5 @@ > MOZ_ASSERT(mContinueCallback); > > + nsCOMPtr<nsIDOMMozSmsMessage> iMessage = do_QueryInterface(aResult); > + if (iMessage) { > + SmsMessage* message = static_cast<SmsMessage*>(aResult); Please do |static_cast<SmsMessage*>(iMessage.get())| here, and same below for iThread. That way we can add more nsISupports base classes to these classes without causing compile failures here.
Attachment #730159 -
Flags: review?(bent.mozilla) → review+
Assignee | ||
Comment 33•12 years ago
|
||
Rebase only.
Attachment #730133 -
Attachment is obsolete: true
Attachment #730691 -
Flags: review+
Assignee | ||
Comment 34•12 years ago
|
||
Rebase to latest patch of bug 838467.
Attachment #730159 -
Attachment is obsolete: true
Attachment #730695 -
Flags: review+
Assignee | ||
Comment 35•12 years ago
|
||
Address review comments in bug 838467 as well.
Attachment #730695 -
Attachment is obsolete: true
Attachment #730824 -
Flags: review+
Assignee | ||
Comment 36•12 years ago
|
||
Rebase.
Attachment #730691 -
Attachment is obsolete: true
Attachment #731483 -
Flags: review+
Assignee | ||
Comment 37•12 years ago
|
||
Address last part of bug 749086 comment 26.
Attachment #730824 -
Attachment is obsolete: true
Attachment #731485 -
Flags: review+
Comment 38•12 years ago
|
||
leo+ as this is a part of MMS. No_UPLIFT for now before the whole MMS is completed
Whiteboard: [target 3/22] → [target 3/22] NO_UPLIFT
Updated•12 years ago
|
Whiteboard: [target 3/22] NO_UPLIFT → [NO_UPLIFT]
Updated•12 years ago
|
Attachment #728111 -
Flags: review?(anygregor) → review+
Comment 39•12 years ago
|
||
Comment on attachment 730137 [details] [diff] [review] Part 4/5: RIL : v3 Review of attachment 730137 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/mobilemessage/src/ril/MobileMessageDatabaseService.js @@ +1914,5 @@ > + this.callback.notifyCursorError(Ci.nsIMobileMessageCallback.INTERNAL_ERROR); > + return; > + } > + > + if (txn) { Can you add a comment explaining when we have a txn and when we have to create one?
Attachment #730137 -
Flags: review?(anygregor) → review+
Assignee | ||
Comment 40•12 years ago
|
||
Rebase.
Attachment #730132 -
Attachment is obsolete: true
Attachment #734586 -
Flags: superreview+
Attachment #734586 -
Flags: review+
Assignee | ||
Comment 41•12 years ago
|
||
Rebase.
Attachment #731483 -
Attachment is obsolete: true
Attachment #734587 -
Flags: review+
Assignee | ||
Comment 42•12 years ago
|
||
Add r=bent only.
Attachment #731485 -
Attachment is obsolete: true
Attachment #734588 -
Flags: review+
Assignee | ||
Comment 43•12 years ago
|
||
1) Address bug 838467 comment 87, use pre-defined constants 2) Address comment 39.
Attachment #730137 -
Attachment is obsolete: true
Attachment #734591 -
Flags: review+
Assignee | ||
Comment 44•12 years ago
|
||
Add r=gwagner only.
Attachment #728111 -
Attachment is obsolete: true
Attachment #734592 -
Flags: review+
Assignee | ||
Comment 45•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ad3f6f75ac8b https://hg.mozilla.org/integration/mozilla-inbound/rev/2957ce54d228 https://hg.mozilla.org/integration/mozilla-inbound/rev/67d1f7ca2381 https://hg.mozilla.org/integration/mozilla-inbound/rev/a0d54f2785fd https://hg.mozilla.org/integration/mozilla-inbound/rev/9b8f074bc865
Assignee | ||
Comment 46•12 years ago
|
||
Update landed patch. Fix android 2.2 debug build.
Attachment #734588 -
Attachment is obsolete: true
Attachment #734960 -
Flags: review+
Comment 47•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ad3f6f75ac8b https://hg.mozilla.org/mozilla-central/rev/2957ce54d228 https://hg.mozilla.org/mozilla-central/rev/67d1f7ca2381 https://hg.mozilla.org/mozilla-central/rev/a0d54f2785fd https://hg.mozilla.org/mozilla-central/rev/9b8f074bc865
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Comment 48•12 years ago
|
||
I'd appreciate it if someone could explain why MozMobileMessageThread doesn't use WebIDL.
Assignee | ||
Comment 49•12 years ago
|
||
(In reply to :Ms2ger from comment #48) > I'd appreciate it if someone could explain why MozMobileMessageThread > doesn't use WebIDL. No special reason, just nobody asked me to rewrite it with WebIDL and I don't know that's a new DOM policy. I've filed bug 859764 for this.
Comment 50•12 years ago
|
||
Unfortunately, I had to back this push out because the test added here was failing frequently. https://hg.mozilla.org/integration/mozilla-inbound/rev/81dc5a203ac2 https://tbpl.mozilla.org/php/getParsedLog.php?id=21591923&tree=Mozilla-Inbound 05:26:41 INFO - TEST-START test_getthreads.js 05:26:50 INFO - /builds/slave/test/build/tests/marionette/tests/dom/mobilemessage/tests/marionette/test_getthreads.js, runTest (marionette_test.MarionetteJSTestCase) ... FAIL 05:26:50 INFO - ====================================================================== 05:26:50 INFO - FAIL: /builds/slave/test/build/tests/marionette/tests/dom/mobilemessage/tests/marionette/test_getthreads.js, runTest (marionette_test.MarionetteJSTestCase) 05:26:50 INFO - ---------------------------------------------------------------------- 05:26:50 INFO - Traceback (most recent call last): 05:26:50 INFO - File "/builds/slave/test/build/tests/marionette/marionette/marionette_test.py", line 228, in runTest 05:26:50 INFO - '%d tests failed:\n%s' % (results['failed'], '\n'.join(fails))) 05:26:50 INFO - AssertionError: 11 tests failed: 05:26:50 INFO - TEST-UNEXPECTED-FAIL | test_getthreads.js | got thread 4, expected thread 2-2 | thread.body 05:26:50 INFO - TEST-UNEXPECTED-FAIL | test_getthreads.js | got 1, expected 0 | thread.unreadCount 05:26:50 INFO - TEST-UNEXPECTED-FAIL | test_getthreads.js | got 5555211003, expected 5555211001 | thread.participants[0] 05:26:50 INFO - TEST-UNEXPECTED-FAIL | test_getthreads.js | got thread 5-2, expected thread 3-2 | thread.body 05:26:50 INFO - TEST-UNEXPECTED-FAIL | test_getthreads.js | got 2, expected 0 | thread.unreadCount 05:26:50 INFO - TEST-UNEXPECTED-FAIL | test_getthreads.js | got 5555211004, expected +15555211002 | thread.participants[0] 05:26:50 INFO - TEST-UNEXPECTED-FAIL | test_getthreads.js | got thread 2-2, expected thread 4 | thread.body 05:26:50 INFO - TEST-UNEXPECTED-FAIL | test_getthreads.js | got 0, expected 1 | thread.unreadCount 05:26:50 INFO - TEST-UNEXPECTED-FAIL | test_getthreads.js | got 5555211001, expected 5555211003 | thread.participants[0] 05:26:50 INFO - TEST-UNEXPECTED-FAIL | test_getthreads.js | got 0, expected 2 | thread.unreadCount 05:26:50 INFO - TEST-UNEXPECTED-FAIL | test_getthreads.js | got +15555211002, expected 5555211004 | thread.participants[0] 05:26:50 INFO - START LOG: 05:26:50 INFO - INFO TEST-START: /builds/slave/test/build/tests/marionette/tests/dom/mobilemessage/tests/marionette/test_getthreads.js Tue Apr 09 2013 12:26:41 GMT+0000 (GMT) 05:26:50 INFO - INFO Validating MozMobileMessageThread attributes ... Tue Apr 09 2013 12:26:48 GMT+0000 (GMT) 05:26:50 INFO - INFO Got thread 51 Tue Apr 09 2013 12:26:48 GMT+0000 (GMT) 05:26:50 INFO - INFO Validating MozMobileMessageThread attributes ... Tue Apr 09 2013 12:26:48 GMT+0000 (GMT) 05:26:50 INFO - INFO Got thread 54 Tue Apr 09 2013 12:26:48 GMT+0000 (GMT) 05:26:50 INFO - INFO Validating MozMobileMessageThread attributes ... Tue Apr 09 2013 12:26:48 GMT+0000 (GMT) 05:26:50 INFO - INFO Got thread 55 Tue Apr 09 2013 12:26:48 GMT+0000 (GMT) 05:26:50 INFO - INFO Validating MozMobileMessageThread attributes ... Tue Apr 09 2013 12:26:48 GMT+0000 (GMT) 05:26:50 INFO - INFO Got thread 52 Tue Apr 09 2013 12:26:48 GMT+0000 (GMT) 05:26:50 INFO - INFO Validating MozMobileMessageThread attributes ... Tue Apr 09 2013 12:26:49 GMT+0000 (GMT) 05:26:50 INFO - INFO Got thread 53 Tue Apr 09 2013 12:26:49 GMT+0000 (GMT) 05:26:50 INFO - INFO Validating MozMobileMessageThread attributes ... Tue Apr 09 2013 12:26:49 GMT+0000 (GMT) 05:26:50 INFO - INFO Got thread 56 Tue Apr 09 2013 12:26:49 GMT+0000 (GMT) 05:26:50 INFO - INFO Validating MozMobileMessageThread attributes ... Tue Apr 09 2013 12:26:49 GMT+0000 (GMT) 05:26:50 INFO - INFO Got thread 57 Tue Apr 09 2013 12:26:49 GMT+0000 (GMT) 05:26:50 INFO - INFO Validating MozMobileMessageThread attributes ... Tue Apr 09 2013 12:26:49 GMT+0000 (GMT) 05:26:50 INFO - INFO Got thread 58 Tue Apr 09 2013 12:26:49 GMT+0000 (GMT) 05:26:50 INFO - INFO Validating whether cursor session ends Tue Apr 09 2013 12:26:49 GMT+0000 (GMT) 05:26:50 INFO - END LOG: 05:26:50 INFO - ---------------------------------------------------------------------- 05:26:50 INFO - Ran 1 test in 9.108s 05:26:50 INFO - FAILED (failures=1)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla23 → ---
Comment 51•12 years ago
|
||
Thanks. It's not really a policy yet, but DOM APIs will need to move away from DOMClassInfo eventually, and I was wondering if there was a particular issue preventing you from using WebIDL immediately.
Assignee | ||
Comment 52•12 years ago
|
||
Remove last patch only: https://hg.mozilla.org/integration/mozilla-inbound/rev/128457f530f2 https://hg.mozilla.org/integration/mozilla-inbound/rev/109cadb6d2e2 https://hg.mozilla.org/integration/mozilla-inbound/rev/625fef901d65 https://hg.mozilla.org/integration/mozilla-inbound/rev/75cd6e698bec
Comment 53•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/128457f530f2 https://hg.mozilla.org/mozilla-central/rev/109cadb6d2e2 https://hg.mozilla.org/mozilla-central/rev/625fef901d65 https://hg.mozilla.org/mozilla-central/rev/75cd6e698bec Nice. I assume the fixed tests will still land at some point.
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Comment 54•12 years ago
|
||
Can someone clarify for me the review and confirmation process? Specifically, I'm unfamiliar with marking something resolved that doesn't have working tests. Any insights?
Comment 55•12 years ago
|
||
Is there an estimate for when this will be ready in the Commercial RIL for v1.1 ? We need this to uplift to b2g18.
Comment 57•12 years ago
|
||
Bug 838467 is ready too so let's go here.
Keywords: checkin-needed
Whiteboard: [NO_UPLIFT]
Comment 58•12 years ago
|
||
Leaving these to Vicamo for the uplift. Also, are the tests still going to be un-horked and landed at some point?
status-b2g18:
--- → affected
status-b2g18-v1.0.0:
--- → wontfix
status-b2g18-v1.0.1:
--- → wontfix
status-firefox21:
--- → wontfix
status-firefox22:
--- → wontfix
status-firefox23:
--- → fixed
Keywords: checkin-needed
Assignee | ||
Comment 59•12 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #58) > Leaving these to Vicamo for the uplift. Also, are the tests still going to > be un-horked and landed at some point? Yes, I'll file another bug for these test cases. Uplift WIP: https://github.com/vicamo/b2g_mozilla-central/tree/bugzilla/838467/b2g18 , verifying.
Comment 60•12 years ago
|
||
(In reply to Vicamo Yang (leaving for Madrid) [:vicamo][:vyang] from comment #59) > (In reply to Ryan VanderMeulen [:RyanVM] from comment #58) > > Leaving these to Vicamo for the uplift. Also, are the tests still going to > > be un-horked and landed at some point? > > Yes, I'll file another bug for these test cases. > > Uplift WIP: > https://github.com/vicamo/b2g_mozilla-central/tree/bugzilla/838467/b2g18 , > verifying. Looking at this and trying to apply it on my local b2g18, I think you forgot to remove nsIDOMSmsRequest.idl from dom/mobilemessage/interfaces/Makefile.in
Assignee | ||
Comment 62•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g18/rev/d303c877d3a0 https://hg.mozilla.org/releases/mozilla-b2g18/rev/ffb4d2086b56 https://hg.mozilla.org/releases/mozilla-b2g18/rev/1b6e432f7f8b https://hg.mozilla.org/releases/mozilla-b2g18/rev/c130c359ea71
Updated•11 years ago
|
Flags: in-moztrap-
You need to log in
before you can comment on or make changes to this bug.
Description
•