Closed
Bug 927724
Opened 11 years ago
Closed 11 years ago
[DSDS][Gaia] Voicemail webapi will be changed for DSDS, so all relevant Apps has to be modified accordingly.
Categories
(Firefox OS Graveyard :: Gaia, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: hsinyi, Assigned: eragonj)
References
Details
Attachments
(1 file, 1 obsolete file)
Before bug 814634, we use, for example
var voicemail = navigator.mozVoicemail;
var number = voicemail.number;
var status = voicemail.status;
After,
var voicemail = navigator.mozVoicemail;
var number = voicemail.getNumber(); or voicemail.getNumber(serviceId);
var status = voicemail.getStatus(); or voicemail.getStatus(serviceId);
Reporter | ||
Comment 1•11 years ago
|
||
This bug is for backward compatibility and new API detection so that bug 814634 won't break gaia.
Updated•11 years ago
|
Assignee: nobody → arthur.chen
Assignee | ||
Updated•11 years ago
|
Assignee: arthur.chen → ejchen
Assignee | ||
Comment 2•11 years ago
|
||
Assignee | ||
Comment 3•11 years ago
|
||
Comment on attachment 820189 [details]
Pointer to Github pull request 13000.html
Hi Fernando, Alive and Arthur,
This is a temporary patch for Gecko Voicemail WebAPI change for future DSDS (Dual Sim Dual Standby) support following our feature detection steps when changing Geck APIs. (you can check here for details : https://bugzilla.mozilla.org/show_bug.cgi?id=887699#c77)
After Gecko merged the patch into codebase, we will definitely remove this part and use our brand-new API instead.
Hope this change make sense to you all, Thanks :)
Attachment #820189 -
Flags: review?(ferjmoreno)
Attachment #820189 -
Flags: review?(arthur.chen)
Attachment #820189 -
Flags: review?(alive)
Comment 4•11 years ago
|
||
Comment on attachment 820189 [details]
Pointer to Github pull request 13000.html
r=me
Attachment #820189 -
Flags: review?(alive) → review+
Comment 5•11 years ago
|
||
Comment on attachment 820189 [details]
Pointer to Github pull request 13000.html
EJ, thanks for the patch. Please also fix the line https://github.com/EragonJ/gaia/blob/065414be66a7123283d3c916e39f123b83d49a99/apps/settings/js/call.js#L719 .
Attachment #820189 -
Flags: review?(arthur.chen)
Assignee | ||
Comment 6•11 years ago
|
||
Comment on attachment 820189 [details]
Pointer to Github pull request 13000.html
Sorry Arthur, I missed one mozVoicemail when grep. Thanks for pointing out this problem.
Attachment #820189 -
Flags: review?(arthur.chen)
Comment 7•11 years ago
|
||
Comment on attachment 820189 [details]
Pointer to Github pull request 13000.html
r=me. Thank you.
Attachment #820189 -
Flags: review?(arthur.chen) → review+
Comment 8•11 years ago
|
||
Comment on attachment 820189 [details]
Pointer to Github pull request 13000.html
Thanks Ej!
Just remember to squash the commits before merging this, please.
Attachment #820189 -
Flags: review?(ferjmoreno) → review+
Assignee | ||
Comment 9•11 years ago
|
||
(In reply to Fernando Jiménez Moreno [:ferjm] (FxAccounts Work Week until Friday) from comment #8)
> Comment on attachment 820189 [details]
> Pointer to Github pull request 13000.html
>
> Thanks Ej!
>
> Just remember to squash the commits before merging this, please.
Yes , I will ! thanks for your reminding, Fernando :)
Assignee | ||
Comment 10•11 years ago
|
||
Thanks all, this patch is landed on master. commit : 4d74b13f20f12996d41eb3e9a6c8775dd1717d67
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 11•11 years ago
|
||
What happens when we don't have the voicemail number information *and* we're on an older gecko?
Looks like we're going to go through the |.getNumber()| call which will badly break the phone.
So we don't really have backward compatibility :/
We might need to back this out.
Flags: needinfo?(ejchen)
Comment 12•11 years ago
|
||
That seems like a valid concern, we might want to check for .getNumber before calling it and fallback to empty string. No need to backout though, this can be done in a follow up.
Comment 13•11 years ago
|
||
Sorry, but this breaks the dialer completely (not able to receive or emit calls...) and Ej is probably asleep.
Backed out: https://github.com/mozilla-b2g/gaia/commit/1164cb2e8c8e4992421f58600e8b896c1882c1fa
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Updated•11 years ago
|
Flags: needinfo?(ejchen)
Comment 16•11 years ago
|
||
In the future - make sure the existing tests in the tree work before we land a patch that risks breaking the dialer & cost control apps. Also - make sure email qa-b2g@mozilla.org if there's a risky patch landing on trunk.
Comment 17•11 years ago
|
||
Even after the backout in Comment 13, the build that Rel Eng respun still show the same issues: See Bug 930804 which I just filed which has this changeset:
Gaia 2feebdb1a2583928f32407d76d798f8654621e2b
SourceStamp 19fd3388c372
BuildID 20131024154023
Version 27.0a1
Assignee | ||
Comment 18•11 years ago
|
||
Thanks Etienne for pointing out this problem. I thought we would have number info no matter in older Gecko or new one, but it seems this is wrong.
Sorry for breaking the others' workflow.
@Jason, I landed this patch after getting green light from Travis, but it's not enough for this patch I think. What you mean emailing `qa-b2g@mozilla.org` ? I can ask for QA's help before landing them (?) Should I needinfo this email or just send a email with details about patches? Is there any workflow for asking `qa-b2g@mozilla.org` help ? Thanks.
Comment 19•11 years ago
|
||
(In reply to EJ Chen [:eragonj] from comment #18)
> Thanks Etienne for pointing out this problem. I thought we would have number
> info no matter in older Gecko or new one, but it seems this is wrong.
>
> Sorry for breaking the others' workflow.
>
> @Jason, I landed this patch after getting green light from Travis, but it's
> not enough for this patch I think. What you mean emailing
> `qa-b2g@mozilla.org` ? I can ask for QA's help before landing them (?)
> Should I needinfo this email or just send a email with details about
> patches? Is there any workflow for asking `qa-b2g@mozilla.org` help ?
> Thanks.
I would email qa-b2g@mozilla.org if you know a patch landing is going to have significant regression risk. That way, if we see immediate problems, we can figure out a way to land the patch without crash landing (e.g. breaking the dialer app). That might mean having someone pick up and sanity test the patch before it lands.
Comment 20•11 years ago
|
||
(In reply to EJ Chen [:eragonj] from comment #18)
> @Jason, I landed this patch after getting green light from Travis, but it's
> not enough for this patch I think.
Yeah that is a bit of a misnomer because Travis cannot run any of the carrier/RIL tests! Maybe Askeing could help you to use the Tpe CI Jenkins to run the gaia-ui-tests against a custom build next time. They have a Hamachi device on it now.
Assignee | ||
Comment 21•11 years ago
|
||
Attachment #820189 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Attachment #823213 -
Flags: review?(arthur.chen)
Assignee | ||
Updated•11 years ago
|
Attachment #823213 -
Flags: review?(alive)
Assignee | ||
Updated•11 years ago
|
Attachment #823213 -
Flags: review?(ferjmoreno)
Assignee | ||
Comment 22•11 years ago
|
||
I have tested this new update from my hamachi and it seems everything is good now. After reviewed by Alive, Fernando and Arthur again, I will ask for `qa-b2g@mozilla.org`'s help. Thanks all.
Comment 23•11 years ago
|
||
Comment on attachment 823213 [details]
New updates
r=me for the settings part. Thanks!
Attachment #823213 -
Flags: review?(arthur.chen) → review+
Updated•11 years ago
|
Attachment #823213 -
Flags: review?(alive) → review+
Updated•11 years ago
|
Attachment #823213 -
Flags: review?(ferjmoreno) → review+
Comment 24•11 years ago
|
||
I did local test w/ gaia base commit d4a1705. Everything works well. Good to go.
Assignee | ||
Comment 25•11 years ago
|
||
(In reply to Al Tsai [:atsai] from comment #24)
> I did local test w/ gaia base commit d4a1705. Everything works well. Good to
> go.
Thanks for your great help, ATsai ! I will merge it into master.
Assignee | ||
Comment 26•11 years ago
|
||
Thanks all, this commit has been merged into gaia/master : 162cc9cd7153dcb284e5f54d5598fb27f1ab18aa.
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Reporter | ||
Updated•11 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•