811192 - Calling setup() in oncall.js is too late caused missing notify info from telephony.

Status: RESOLVED FIXED

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

Description

[Vincent Liu[:vliu]]

When I tried to dig into a dialer problem in partner's platform , I found the init sequence in oncall.js have some problems caused dialer problem in partner's platform. In otoro, it also had potential issue in it. Let me explain what I knew till now.

1. When operator pressed phone number and dialed, oncall.js is load and waited for localized event to call OnCallHandler.setup(). Please see the below link to know the code position I said.
https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/oncall.js#L540
   In otoro, the time duration between called openCallScreen() in dialer.js and received localized event in oncall.js is about 2~3 seconds. During this time period, it lost any notify event from telephony. Does it the reason why setup() need to invoke onCallsChanged() at once?
https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/oncall.js#L214

2. In setup(), if handledCalls.length got zero in L214, it then called exitCallScreen(false) to terminate. It is weird because oncall.js only had one chance to get call info from telephony in setup(). In other words, if telephony is not ready for call info at this moment, oncall.js never had chance to get. Actually it happens in partner's platform.
https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/oncall.js#L219

Comment 1

[Vincent Liu[:vliu]]

Attachment: Simple patch as work around to fix this issue.

Hi Etienne, 
May I got change to get your comment for my question or what I saw? I attached a simple patch as work around to fix this issue. I knew this simple patch not to consider too much for the whole dialer app's code structure. I just want to point out the problem instead.

Comment 2

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

Line215 'telephony.oncallschanged = onCallsChanged();' seems to appear quite late, so it might miss some early 'callschanged' events for 'dialing' and 'incoming' call state. Can't we add event listener as early as possible, for example, right after obtaining mozTelephony [1]? I'm not sure whether the implementation here was designed on purpose for a specific reason that I haven't realized yet. Please correct me because I might miss something here. :)  

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/oncall.js#L159

Comment 3

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

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #2)
> Line215 'telephony.oncallschanged = onCallsChanged();' seems to appear quite
> late, so it might miss some early 'callschanged' events for 'dialing' and
> 'incoming' call state. Can't we add event listener as early as possible, for
> example, right after obtaining mozTelephony [1]? I'm not sure whether the
> implementation here was designed on purpose for a specific reason that I
> haven't realized yet. Please correct me because I might miss something here.
> :)  
> 
> [1]
> https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/
> js/oncall.js#L159

A-ha, vliu just submitted a patch that I had in mind :)

Comment 4

[Etienne Segonzac (:etienne)]

Comment on attachment 680934 [details] [diff] [review]
Simple patch as work around to fix this issue.

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

::: apps/communications/dialer/js/oncall.js
@@ +242,5 @@
> +    // If the call was ended before we got here we can close
> +    // right away.
> +    if (handledCalls.length === 0) {
> +      exitCallScreen(false);
> +    }

This was originally added to gracefully handle cases with no network at all, we should now have a prompt for this.

Did you ran some tests with no simcard too?

Comment 5

[Vincent Liu[:vliu]]

(In reply to Etienne Segonzac (:etienne) from comment #4)
> Comment on attachment 680934 [details] [diff] [review]
> Simple patch as work around to fix this issue.
> 
> Review of attachment 680934 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: apps/communications/dialer/js/oncall.js
> @@ +242,5 @@
> > +    // If the call was ended before we got here we can close
> > +    // right away.
> > +    if (handledCalls.length === 0) {
> > +      exitCallScreen(false);
> > +    }
> 
> This was originally added to gracefully handle cases with no network at all,
> we should now have a prompt for this.
> 
> Did you ran some tests with no simcard too?

Hi Etienne,
May I know what will happens if this pieces of code is taken off in originally code design? In my patch, I put this pieces of code in onCallsChanged(). I think it is also work for length checking. I just tried calling emergency call with my patch under no simcard, it works normally. Besides this, I listed some questions in Description field. Can you please give me any comment if you know any? Thanks.

Comment 6

[Etienne Segonzac (:etienne)]

(In reply to vliu from comment #5)
> May I know what will happens if this pieces of code is taken off in
> originally code design? 

In some cases the call was ended before the call sreen was loaded/the callschanged callback set.
So the call screen was "stuck" open.

> Besides this, I listed some questions in Description field. Can you please
> give me any comment if you know any? Thanks.

I'm not sure I understand the questions correctly...

Comment 7

[Vincent Liu[:vliu]]

(In reply to Etienne Segonzac (:etienne) from comment #6)

> In some cases the call was ended before the call sreen was loaded/the
> callschanged callback set.
> So the call screen was "stuck" open.
> 

I am afraid it also have side effect on current code implementation. If any code info in telephony is not ready before length checking, setup() will call exitCallScreen(false) to close the close screen. In this case, telephony spent more time than call screen loaded time.

Comment 8

[Vincent Liu[:vliu]]

> I am afraid it also have side effect on current code implementation. If any
> code info in telephony is not ready before length checking, setup() will
> call exitCallScreen(false) to close the close screen. In this case,
> telephony spent more time than call screen loaded time.

Actually this issue happens in partner's platform. In current code implementation, the timing the telephone object got call info running after the length checking. It caused the call screen was shutdown immediately even this call had been established.

Comment 9

[Vincent Liu[:vliu]]

(In reply to Etienne Segonzac (:etienne) from comment #6)

> > Besides this, I listed some questions in Description field. Can you please
> > give me any comment if you know any? Thanks.
> 
> I'm not sure I understand the questions correctly...

Hi Etienne,
Let me tried to explain more detail about my concern for potential issue in oncall.js in otoro. When operator made a call, the oncall.js was loaded and then setup() was invoked for initialization. 

  function setup() {
    if (telephony) {
      // Somehow the muted property appears to true after initialization.
      // Set it to false.
      telephony.muted = false;

      // Needs to be called at least once
      onCallsChanged();                             // Check point 1
      telephony.oncallschanged = onCallsChanged;    // Check point 2            

      // If the call was ended before we got here we can close
      // right away.
      if (handledCalls.length === 0) {              // Check point 3 
        exitCallScreen(false);
      }
    }
  }

If we looked into setup() at Check point 1, it firstly forced calling onCallsChanged(). If telephony was not ready for call info at this moment, the onCallsChanged() got nothing. In other words, handledCalls.length would be 0. Based on above, the length checking at Point 3 would cause it run exitCallScreen(false) to exit the call screen. This really happens in partner's platform and also have potential issue in otoro. How do you think?

I could understand the problem why you add length checking in setup(). But in this implementation, it caused the above side effect. How do you think?

Comment 10

[Etienne Segonzac (:etienne)]

(In reply to vliu from comment #9)
> If we looked into setup() at Check point 1, it firstly forced calling
> onCallsChanged(). If telephony was not ready for call info at this moment,
> the onCallsChanged() got nothing. In other words, handledCalls.length would
> be 0. Based on above, the length checking at Point 3 would cause it run
> exitCallScreen(false) to exit the call screen. This really happens in
> partner's platform and also have potential issue in otoro. How do you think?
> 
> I could understand the problem why you add length checking in setup(). But
> in this implementation, it caused the above side effect. How do you think?

Thanks for the clarification. Let's recap.

We need to account for things both ways, currently the code handle those cases:
- call is already connected at the time we register telephony.oncallschanged -> we need to call onCallsChanged() at least once since telephony.oncallschanged won't be triggered before the call is disconnected
- call is already _dis_-conected (yes it happens) at the time we register telephony.oncallschanged -> we need to call onCallsChanged at least once and an exit case when telephony.calls.length == 0 otherwise the call screen will be left open! (no telephony.oncallschanged will be triggered)

The issue you're reporting is:
- telephony.calls is not yet ready when we call onCallsChanged() the first time so we hit the exit case.

We need a way to fix this without breaking the 2 previous cases.

Do we have a clean way to know if telephony "isn't ready for call info yet"?

Comment 11

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

(In reply to Etienne Segonzac (:etienne) from comment #10)
> (In reply to vliu from comment #9)
> > If we looked into setup() at Check point 1, it firstly forced calling
> > onCallsChanged(). If telephony was not ready for call info at this moment,
> > the onCallsChanged() got nothing. In other words, handledCalls.length would
> > be 0. Based on above, the length checking at Point 3 would cause it run
> > exitCallScreen(false) to exit the call screen. This really happens in
> > partner's platform and also have potential issue in otoro. How do you think?
> > 
> > I could understand the problem why you add length checking in setup(). But
> > in this implementation, it caused the above side effect. How do you think?
> 
> Thanks for the clarification. Let's recap.
> 
> We need to account for things both ways, currently the code handle those
> cases:
> - call is already connected at the time we register telephony.oncallschanged

This is true.

> -> we need to call onCallsChanged() at least once since
> telephony.oncallschanged won't be triggered before the call is disconnected

No, it will be triggered for 'dialing', 'incoming' and 'disconnected.' In oncall.js, we use 'window.navigator.mozTelephony' to obtain 'telephony' that creates another 'telephony object' which is different from the one we got in dialer.js. At the moment it is created, the new telephony object will ask RIL for enumerating all the calls. Then the telephony object will dispatch 'callschanged' event.


> - call is already _dis_-conected (yes it happens) at the time we register
> telephony.oncallschanged -> we need to call onCallsChanged at least once and
> an exit case when telephony.calls.length == 0 otherwise the call screen will
> be left open! (no telephony.oncallschanged will be triggered)

I got it. This is really tricky...

> 
> The issue you're reporting is:
> - telephony.calls is not yet ready when we call onCallsChanged() the first
> time so we hit the exit case.
> 
> We need a way to fix this without breaking the 2 previous cases.
> 
> Do we have a clean way to know if telephony "isn't ready for call info yet"?
In the current architecture of Dialer, I would say 'oncallschanged'.

Comment 12

[Vincent Liu[:vliu]]

> 
> The issue you're reporting is:
> - telephony.calls is not yet ready when we call onCallsChanged() the first
> time so we hit the exit case.
> 
> We need a way to fix this without breaking the 2 previous cases.
> 
> Do we have a clean way to know if telephony "isn't ready for call info yet"?
> In the current architecture of Dialer, I would say 'oncallschanged'.

Hi Etienne, Hsin-Yi,
May I propose an idea for this issue. We create a dom API in telephone object to get current call state. I know we have oncallschanged to notify this, but now it seem not working in some cases Etienne talked in comment 10. Besides length checking, we can also sniff call state immediately to know call state in Telephone object to decide if we want to exit screen or not.  

Case 1:
- call is already connected at the time we register telephony.oncallschanged -> we need to call onCallsChanged() at least once since telephony.oncallschanged won't be triggered before the call is disconnected

I am not sure why telephony.oncallschanged won't be triggered before the call is disconnected. It should be triggered when call state changed. In this case, I guess the state we sniffed is in "connected" state and the length checking is 1. the call screen shouldn't be closed.

Case 2:
- call is already _dis_-conected (yes it happens) at the time we register telephony.oncallschanged -> we need to call onCallsChanged at least once and an exit case when telephony.calls.length == 0 otherwise the call screen will be left open! (no telephony.oncallschanged will be triggered)

I tried to make a call and quickly terminate this call to reproduce this problem but it seem not the correct way to reproduce it. But I guess when this issue happens, the call state is "disconnect" when we sniff it at the time when we do length checking. In this case, the call screen should be closed.

Case 3:
- telephony.calls is not yet ready when we call onCallsChanged() the first time so we hit the exit case.

This issue happens in partner's platform. I guess when do length checking, the call state should not be in "disconnected" because Telephony still wait for call info. In this case, we shouldn't be closed.

May I have chance to ask for you idea if it works for covering all cases. Thanks

Comment 13

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

(In reply to vliu from comment #12)
> > 
> > The issue you're reporting is:
> > - telephony.calls is not yet ready when we call onCallsChanged() the first
> > time so we hit the exit case.
> > 
> > We need a way to fix this without breaking the 2 previous cases.
> > 
> > Do we have a clean way to know if telephony "isn't ready for call info yet"?
> > In the current architecture of Dialer, I would say 'oncallschanged'.
> 
> Hi Etienne, Hsin-Yi,
> May I propose an idea for this issue. We create a dom API in telephone
> object to get current call state. I know we have oncallschanged to notify
> this, but now it seem not working in some cases Etienne talked in comment
> 10. Besides length checking, we can also sniff call state immediately to
> know call state in Telephone object to decide if we want to exit screen or
> not.  
> 

Hi vliu,

As we discussed earlier, when there's no call (calls.length == 0), then there's surely no available information about 'call state.' I am afraid that this proposal doesn't solve the exitCallScreen issue.

> Case 1:
> - call is already connected at the time we register telephony.oncallschanged
> -> we need to call onCallsChanged() at least once since
> telephony.oncallschanged won't be triggered before the call is disconnected
> 
> I am not sure why telephony.oncallschanged won't be triggered before the
> call is disconnected. It should be triggered when call state changed. 

Please see comment 11.

> In this case, I guess the state we sniffed is in "connected" state and the
> length checking is 1. the call screen shouldn't be closed.
>

Comment 14

[Etienne Segonzac (:etienne)]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #11)
> (In reply to Etienne Segonzac (:etienne) from comment #10)
> > (In reply to vliu from comment #9)
> > > If we looked into setup() at Check point 1, it firstly forced calling
> > > onCallsChanged(). If telephony was not ready for call info at this moment,
> > > the onCallsChanged() got nothing. In other words, handledCalls.length would
> > > be 0. Based on above, the length checking at Point 3 would cause it run
> > > exitCallScreen(false) to exit the call screen. This really happens in
> > > partner's platform and also have potential issue in otoro. How do you think?
> > > 
> > > I could understand the problem why you add length checking in setup(). But
> > > in this implementation, it caused the above side effect. How do you think?
> > 
> > Thanks for the clarification. Let's recap.
> > 
> > We need to account for things both ways, currently the code handle those
> > cases:
> > - call is already connected at the time we register telephony.oncallschanged
> 
> This is true.
> 
> > -> we need to call onCallsChanged() at least once since
> > telephony.oncallschanged won't be triggered before the call is disconnected
> 
> No, it will be triggered for 'dialing', 'incoming' and 'disconnected.' In
> oncall.js, we use 'window.navigator.mozTelephony' to obtain 'telephony' that
> creates another 'telephony object' which is different from the one we got in
> dialer.js. At the moment it is created, the new telephony object will ask
> RIL for enumerating all the calls. Then the telephony object will dispatch
> 'callschanged' event.
> 

Was it always like this?
If we're guaranteed that the telephony object will get all the callschanged events even if the call is already connected we can probably safely handle all the cases like this.

(to reproduce this you can setTimeout the window.open call in dialer.js a bit, we first saw this case on otoro when we were still using appcache)

Comment 15

[Vincent Liu[:vliu]]

(In reply to Etienne Segonzac (:etienne) from comment #14)

> Was it always like this?
> If we're guaranteed that the telephony object will get all the callschanged
> events even if the call is already connected we can probably safely handle
> all the cases like this.
> 

Hi Etienne,
I checked the code and we have the below code flow. 

1. In dialer.js, we created the first telephony object for dialer apps.
https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/dialer.js#L4

2. We called dial function in the first telephony object.
https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/dialer.js#L126

3. Run window.open to load oncall.js

4. In oncall.js, we created the second telephony object for oncall.
https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/oncall.js#L159

The thing is we called dial in the first telephony object and did length checking in the second telephony object. For this, I think there had chance we couldn't get oncallschanged
trigged in the second telephony object.

There was a question came on my mind. Why do we need oncall.js to deal with all call events? If we put all things in dialer.js, maybe we won't miss any notification from telephony because we only have one telephony object to deal. How do you think?

Comment 16

[Etienne Segonzac (:etienne)]

(In reply to vliu from comment #15)
> There was a question came on my mind. Why do we need oncall.js to deal with
> all call events? If we put all things in dialer.js, maybe we won't miss any
> notification from telephony because we only have one telephony object to
> deal. How do you think?

All the multiple call support lives in oncall.js....

Comment 17

[Vincent Liu[:vliu]]

(In reply to Etienne Segonzac (:etienne) from comment #16)

> All the multiple call support lives in oncall.js....

Got it. Thanks for your infomation.


> - call is already _dis_-conected (yes it happens) at the time we register
> telephony.oncallschanged -> we need to call onCallsChanged at least once and
> an exit case when telephony.calls.length == 0 otherwise the call screen will
> be left open! (no telephony.oncallschanged will be triggered)
> 

Hi Etienne,

I think this case just the same in comment 6, right? I know your word but I don't have a method to reproduce this problem. Can you offer me more info to let me reproduce it? I hope this would help me coming out some ideas to cover all cases. Thanks.

Comment 18

[Vincent Liu[:vliu]]

(In reply to Etienne Segonzac (:etienne) from comment #10)
> (In reply to vliu from comment #9)
> > If we looked into setup() at Check point 1, it firstly forced calling
> > onCallsChanged(). If telephony was not ready for call info at this moment,
> > the onCallsChanged() got nothing. In other words, handledCalls.length would
> > be 0. Based on above, the length checking at Point 3 would cause it run
> > exitCallScreen(false) to exit the call screen. This really happens in
> > partner's platform and also have potential issue in otoro. How do you think?
> > 
> > I could understand the problem why you add length checking in setup(). But
> > in this implementation, it caused the above side effect. How do you think?
> 
> Thanks for the clarification. Let's recap.
> 
> We need to account for things both ways, currently the code handle those
> cases:
> - call is already connected at the time we register telephony.oncallschanged
> -> we need to call onCallsChanged() at least once since
> telephony.oncallschanged won't be triggered before the call is disconnected
> - call is already _dis_-conected (yes it happens) at the time we register
> telephony.oncallschanged -> we need to call onCallsChanged at least once and
> an exit case when telephony.calls.length == 0 otherwise the call screen will
> be left open! (no telephony.oncallschanged will be triggered)
> 
> The issue you're reporting is:
> - telephony.calls is not yet ready when we call onCallsChanged() the first
> time so we hit the exit case.
> 
> We need a way to fix this without breaking the 2 previous cases.
> 
> Do we have a clean way to know if telephony "isn't ready for call info yet"?

Hi Etienne,

Did you got any good idea for these issues? Or what info I need to offer?

Comment 19

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

Attachment: WIP v1

This WIP is a proposal for addressing comment 10. 
-Issue 1: as explained in comment 11, we don't need to call onCallsChanged() at least once. We should rather wait for 'callschanged' event. In this WIP, we register 'callschanged' event at the most possibly early moment. We enter onCallsChanged() when we receive 'callschanged', such that calls will be ready in onCallsChanged(). Hopefully this resolves vliu's original reported issue.

-Issue 2 (callscreen stuck open): since a) dialer.js launches callscreen , and b) the lifetime of 'dialer.js' is longer than (at least, as long as) a call, i.e. it doesn't miss the events of calls, I am thinking that 'dialer.js' postMessage to notify callscreen of exiting when there are no calls. This method makes sure that we do exit call screen only when there really exist no calls.

vliu and Etienne,
How do you think about this WIP? Did I miss something? Thank you! :)

Comment 20

[Etienne Segonzac (:etienne)]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #19)
> Created attachment 685503 [details] [diff] [review]
> WIP v1
> 
> This WIP is a proposal for addressing comment 10. 
> -Issue 1: as explained in comment 11, we don't need to call onCallsChanged()
> at least once. We should rather wait for 'callschanged' event. In this WIP,
> we register 'callschanged' event at the most possibly early moment. We enter
> onCallsChanged() when we receive 'callschanged', such that calls will be
> ready in onCallsChanged(). Hopefully this resolves vliu's original reported
> issue.
> 
> -Issue 2 (callscreen stuck open): since a) dialer.js launches callscreen ,
> and b) the lifetime of 'dialer.js' is longer than (at least, as long as) a
> call, i.e. it doesn't miss the events of calls, I am thinking that
> 'dialer.js' postMessage to notify callscreen of exiting when there are no
> calls. This method makes sure that we do exit call screen only when there
> really exist no calls.
> 
> vliu and Etienne,
> How do you think about this WIP? Did I miss something? Thank you! :)

Francisco is currently refactoring the postMessage for the dialer a bit (bug 809704) , but the general idea sounds really good!

Comment 21

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

Attachment: WIP v2

WIP v2

The main concept is as comment 19. Per vliu's opinions, we should be careful of the case that 'call is ended before callscreen is loaded.' Otherwise, callscreen may still miss the message posted by dialer.js. This patch addresses that. Thank you, vliu.

Comment 22

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

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #21)
> Created attachment 685534 [details] [diff] [review]
> WIP v2
> 
> WIP v2
> 
> The main concept is as comment 19. Per vliu's opinions, we should be careful
> of the case that 'call is ended before callscreen is loaded.' Otherwise,
> callscreen may still miss the message posted by dialer.js. This patch
> addresses that. Thank you, vliu.

Hi vliu, could you please help test this patch with partner's platform? Thank you.

Comment 23

[Vincent Liu[:vliu]]

Hi Hsin-Yi,

Sure, No problem. I will do it. :)

Comment 24

[Vincent Liu[:vliu]]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #22)
> (In reply to Hsin-Yi Tsai [:hsinyi] from comment #21)
> > Created attachment 685534 [details] [diff] [review]
> > WIP v2
> > 
> > WIP v2
> > 
> > The main concept is as comment 19. Per vliu's opinions, we should be careful
> > of the case that 'call is ended before callscreen is loaded.' Otherwise,
> > callscreen may still miss the message posted by dialer.js. This patch
> > addresses that. Thank you, vliu.
> 
> Hi vliu, could you please help test this patch with partner's platform?
> Thank you.


Hi Hsin-Yi,

Your patch also works on partner's platform. Thanks for your help. Can you please let your patch enter review process? Thanks.(In reply to vliu from comment #23)

Comment 25

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

(In reply to vliu from comment #24)
> (In reply to Hsin-Yi Tsai [:hsinyi] from comment #22)
> > (In reply to Hsin-Yi Tsai [:hsinyi] from comment #21)
> > > Created attachment 685534 [details] [diff] [review]
> > > WIP v2
> > > 
> > > WIP v2
> > > 
> > > The main concept is as comment 19. Per vliu's opinions, we should be careful
> > > of the case that 'call is ended before callscreen is loaded.' Otherwise,
> > > callscreen may still miss the message posted by dialer.js. This patch
> > > addresses that. Thank you, vliu.
> > 
> > Hi vliu, could you please help test this patch with partner's platform?
> > Thank you.
> 
> 
> Hi Hsin-Yi,
> 
> Your patch also works on partner's platform. Thanks for your help. Can you
> please let your patch enter review process? Thanks.(In reply to vliu from
> comment #23)

Thank you :)

Comment 26

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

Attachment: Pull request

Changes made in this patch are explained in comment 19 and comment 21. Thanks!

Comment 27

[Etienne Segonzac (:etienne)]

Comment on attachment 685985 [details]
Pull request

We refactored the communication between the app window and the call screen, so we should use this code path for the postMessages added in this patch.
(details on github)

The rest looks good, thanks for fixing those edge cases bugs!

Comment 28

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

Comment on attachment 685985 [details]
Pull request

Comment addressed. Ask for the second review. Thanks!

Comment 29

[Etienne Segonzac (:etienne)]

https://github.com/mozilla-b2g/gaia/commit/43a53fa92241e915a38593e6da79a3f6d749e845

Comment 30

[Etienne Segonzac (:etienne)]

Thanks again for the tracking down this bug!