Gene Lian (I already quit Mozilla) Reporter
	Description • 12 years ago

+++ 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.

	Gene Lian (I already quit Mozilla) 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.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 2 • 12 years ago

Will create a DOMCursor based getThreads() API instead.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 4 • 12 years ago

DOM & IPC done, just have to implement MobileMessageThread & fix backends.

	Vicamo Yang [:vicamo][:vyang] 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.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 11 • 12 years ago

Keep the same attribute type and naming as bug 850127.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 12 • 12 years ago

Keep the same attribute type and naming as bug 850127.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 13 • 12 years ago

Extract common validations and document the test cases more.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 14 • 12 years ago

Can we mark it as leo+ now? It's also blocking 847744, which is already a leo+.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 15 • 12 years ago

Rebase to m-c tip to include MMS changes

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 16 • 12 years ago

1) Rebase
2) follow variable naming in bug 838467

	Vicamo Yang [:vicamo][:vyang] 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

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 18 • 12 years ago

Rebase only.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 19 • 12 years ago

Refactor. Much much cleaner and well documented.

	Mounir Lamouri (: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

	Mounir Lamouri (:mounir)
	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.

	Vicamo Yang [:vicamo][:vyang] 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 :)

	Vicamo Yang [:vicamo][:vyang] 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 :)

	Blake Kaplan (:mrbkap) (inactive)
	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.

	Mounir Lamouri (:mounir)
	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 ;)

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 27 • 12 years ago

Address review comment 21.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 28 • 12 years ago

Address review comment 22 and comment 25.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 29 • 12 years ago

Address review comments in bug 838467.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 30 • 12 years ago

inline createThreadFromRecord() because it's only called in GetThreadsCursor.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 31 • 12 years ago

The last patch doesn't address bug 838467 comment 57. Fixed.

	Ben Turner (not reading bugmail, use the needinfo flag!)
	Comment 32 • 12 years ago

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.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 33 • 12 years ago

Rebase only.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 34 • 12 years ago

Rebase to latest patch of bug 838467.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 35 • 12 years ago

Address review comments in bug 838467 as well.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 36 • 12 years ago

Rebase.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 37 • 12 years ago

Address last part of bug 749086 comment 26.

	Joe Cheng [:jcheng] (please needinfo)
	Comment 38 • 12 years ago

leo+ as this is a part of MMS. No_UPLIFT for now before the whole MMS is completed

	Gregor Wagner [:gwagner]
	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?

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 40 • 12 years ago

Rebase.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 41 • 12 years ago

Rebase.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 42 • 12 years ago

Add r=bent only.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 43 • 12 years ago

1) Address bug 838467 comment 87, use pre-defined constants
2) Address comment 39.

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 44 • 12 years ago

Add r=gwagner only.

	Vicamo Yang [:vicamo][:vyang] 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

	Vicamo Yang [:vicamo][:vyang] Assignee
	Comment 46 • 12 years ago

Update landed patch. Fix android 2.2 debug build.

	Ed Morley [:emorley]
	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

	:Ms2ger (he/him; ⌚ UTC+1/+2)
	Comment 48 • 12 years ago

I'd appreciate it if someone could explain why MozMobileMessageThread doesn't use WebIDL.

	Vicamo Yang [:vicamo][:vyang] 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.

	Ryan VanderMeulen [:RyanVM]
	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)

	:Ms2ger (he/him; ⌚ UTC+1/+2)
	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.

	Vicamo Yang [:vicamo][:vyang] 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

	Ryan VanderMeulen [:RyanVM]
	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.

	Rick Waldron [:rwaldron]
	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?

	Julien Wajsberg [:julienw]
	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.

	Alhad Purnapatre
	Comment 56 • 12 years ago

This should be ok for uplift from Commercial RIL POV.

	Julien Wajsberg [:julienw]
	Comment 57 • 12 years ago

Bug 838467 is ready too so let's go here.

	Ryan VanderMeulen [:RyanVM]
	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?

	Vicamo Yang [:vicamo][:vyang] 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.

	:gerard-majax [PTO 01/08/2023-10/09/2023]
	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

	Julien Wajsberg [:julienw]
	Comment 61 • 12 years ago

Do you think you'll be able to uplift this soon ?

	Vicamo Yang [:vicamo][:vyang] 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