969280 - [Dialer] dialer app modification according to Telephony API change, i.e. dial() returning a Promise

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Dialer, defect)

Description

[Hsin-Yi Tsai (she/her) [:hsinyi]]

In bug 889737, we came to agreement that we should have telephony.dial() return a Promise.

This bug is filed for corresponding gaia/dialer work due to that API change.

Comment 1

[Hsin-Yi Tsai (she/her) [:hsinyi]]

nominate since 1.4+ MMI features cannot be completed without this bug.

Comment 2

[Szu-Yu Chen [:aknow] (inactive)]

In order to have the modification smoothly, following step is proposed.
1. Gaia should first provide a version that is backward compatible, i.e, support both TelephonyCall and Promise return by API call.
2. Gecko change the interface.
3. Gaia remove the original support of 'TelephonyCall'. Now always use the promise.

Comment 3

[Szu-Yu Chen [:aknow] (inactive)]

(In reply to Szu-Yu Chen [:aknow] from comment #2)
> In order to have the modification smoothly, following step is proposed.
> 1. Gaia should first provide a version that is backward compatible, i.e,
> support both TelephonyCall and Promise return by API call.

Need a bug to address item 1.
We cannot land the gecko change until we have a gaia that could support both old and new version of API.

Comment 4

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Szu-Yu Chen [:aknow] from comment #3)
> (In reply to Szu-Yu Chen [:aknow] from comment #2)
> > In order to have the modification smoothly, following step is proposed.
> > 1. Gaia should first provide a version that is backward compatible, i.e,
> > support both TelephonyCall and Promise return by API call.
> 
> Need a bug to address item 1.
> We cannot land the gecko change until we have a gaia that could support both
> old and new version of API.

I filed this bug for item 1.

Comment 5

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Hi Joe,

Bug 969218 (1.4 blocker) would be ready very soon. As that gecko bug cannot be landed before gaia is ready for the new API, we need gaia work being proceeded in parallel to not block other following issues. Need Comms' team's help :)

Comment 6

[Anthony Ricaud (:rik)]

I'll take this. For future reference, attachment 8373839 [details] [diff] [review] is very helpful.

Is it possible to have a Gecko build to flash? Or a branch? It's usually easier to work with that rather than applying possibly bitrotted patches. Thanks!

Comment 7

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Yeah, thanks for Rik's help. :)

Aknow, could you please provide a github working branch for Rik?

Comment 8

[Szu-Yu Chen [:aknow] (inactive)]

Hi :rik,
You could refer to this branch for gecko/.
https://github.com/aknow/gecko-dev/tree/promise_dial

Comment 9

[Joe Cheng [:jcheng] (please needinfo)]

hey Rik, how much of an effort do you think this will be? it seems to be a big change so want to be careful on taking this for 1.4. Thanks

Comment 10

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Correct the dependency. This should theoretically be landed before bug 969218.

Comment 11

[Anthony Ricaud (:rik)]

Thankfully, we're only using telephony.dial and dialEmergency in 4 places. I'll take this in this sprint.

Comment 12

[Joe Cheng [:jcheng] (please needinfo)]

bug 889737 does not seem to be blocking anymore, let's not block 1.4 with this.

Comment 13

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Joe Cheng [:jcheng] from comment #12)
> bug 889737 does not seem to be blocking anymore, let's not block 1.4 with
> this.

however this blocks bug 969218, which blocks a QC requirement. We should have this in 1.4+.

Comment 14

[Anthony Ricaud (:rik)]

Just to clarify, based on comment 13, I'm working on this even though it's not marked as 1.4+.

Comment 15

[Anthony Ricaud (:rik)]

I have a patch and tests for the Dialer part. I need another one for the emergency pad and the voicemail notification.

Comment 16

[Kai-Chih Hu [:khu]]

(In reply to Anthony Ricaud (:rik) from comment #15)
> I have a patch and tests for the Dialer part. I need another one for the
> emergency pad and the voicemail notification.

Can we land this bug before 2/28?

Comment 17

[Anthony Ricaud (:rik)]

Attachment: https://github.com/mozilla-b2g/gaia/pull/16656

I've tested this on a single SIM with trunk gecko and the WIP gecko from aknow.
I'll test on a DSDS device just to be sure but asking for review already.

Comment 18

[Etienne Segonzac (:etienne)]

Comment on attachment 8382075 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/16656

A clean patch if I ever saw one :)

r=me with the promise edition for the |emergency-call/dialer_test.js|

Note: I could only do manual testing on a pre-patch single sim device, since flashing aknow's patch [1] only gets me a black screen on my keon.

[1] https://github.com/aknow/gecko-dev/compare/promise_dial.patch

Comment 19

[Anthony Ricaud (:rik)]

Tada on master!

https://github.com/mozilla-b2g/gaia/commit/52ad4108920c48d68565992a31fc3901ab5168c3

Comment 20

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to revert this in https://github.com/mozilla-b2g/gaia/commit/548965a5866aa38b5f44c336b3a7fb723164e277 because this apparently broke gaia-unit tests in tbpl:
https://tbpl.mozilla.org/php/getParsedLog.php?id=35309266&tree=B2g-Inbound

Comment 21

[Anthony Ricaud (:rik)]

Shit :( This bug si blocking work for folks in Taipei and we have an aggressive deadline. Is it possible to re-land but without the offending file? This is a new test file. I don't have a computer handy to do it myself.

I'll obviously investigate the failure tomorrow.

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

That'd just be unreverting it and deleting  apps/system/test/unit/emergency-call/dialer_test.js, right?

Comment 23

[Anthony Ricaud (:rik)]

Yup! Thanks!

Comment 24

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Relanded in https://github.com/mozilla-b2g/gaia/commit/98001aa1906ce97d4fa13c9109f95c9f1bdd1a12
dialer_test.js removed in https://github.com/mozilla-b2g/gaia/commit/fd12f93849af911ae80940195058e4277c7dc483

Comment 25

[Michael Henretty [:mikehenrty][:mhenretty]]

Just FYI, a merge conflict in dialer_tests.js caused the tests to be re-added, so I re-removed it:
https://github.com/mozilla-b2g/gaia/commit/22d48b62df7901ad45044f66e15e7d8943884a06

I guess we should file a follow up to re-add the tests.

Comment 26

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Thank you guys!