[Statusbar] Signal icon is not animated while searching

RESOLVED FIXED in 2.2 S14 (12june)

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: apastor, Assigned: apastor)

Tracking

({regression})

unspecified
2.2 S14 (12june)
ARM
Gonk (Firefox OS)
regression

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [systemsfe])

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

4 years ago
STR:

1.- Start the phone and insert the PIN

Expected:

The signal icon gets animated while searching signal. Then it stops with as many bars as the signal offers.

Actual:

The signal icon is static with 0 bars
(Assignee)

Comment 1

4 years ago
Regression of bug 1151502. :gmarty, could you please take a look?
Flags: needinfo?(gmarty)
Keywords: regression
Assignee: nobody → gmarty
Target Milestone: --- → 2.2 S14 (12june)
Blocks: 1151502
Created attachment 8620928 [details] [review]
Github PR

I reworked the logic for the searching state.
What do you think, Alberto?
Flags: needinfo?(gmarty)
Attachment #8620928 - Flags: review?(apastor)
(Assignee)

Comment 3

4 years ago
Mhm, I'm not completely sure about the solution here. In my opinion, the code was right before bug 1151502 landed. In theory, we should receive an 'update' event once is connected, and that event already calls updateSignal. If that's not updating the icon correctly, we need to investigate why, but adding more conditions to the updateSignal method doesn't seem correct to me.

The other option is moving all the 'searching' logic inside the updateSignal method (which I think is your idea), but then I would reestructurate the 'update' method [0]. I wouldn't set the 'searching' dataset there, then [1], for example.

Asking for a second opinion. Etienne?

[0] https://github.com/gmarty/gaia/blob/Bug-1171883-Signal-icon-is-not-animated-while-searching/apps/system/js/signal_icon.js#L28

[1] https://github.com/gmarty/gaia/blob/Bug-1171883-Signal-icon-is-not-animated-while-searching/apps/system/js/signal_icon.js#L84
Flags: needinfo?(etienne)
(In reply to Alberto Pastor [:albertopq] from comment #3)
> Mhm, I'm not completely sure about the solution here. In my opinion, the
> code was right before bug 1151502 landed. In theory, we should receive an
> 'update' event once is connected, and that event already calls updateSignal.
> If that's not updating the icon correctly, we need to investigate why, but
> adding more conditions to the updateSignal method doesn't seem correct to me.
> 
> The other option is moving all the 'searching' logic inside the updateSignal
> method (which I think is your idea), but then I would reestructurate the
> 'update' method [0]. I wouldn't set the 'searching' dataset there, then [1],
> for example.
> 
> Asking for a second opinion. Etienne?

Correct me if I'm wrong but looks like [0] is the key change in Guillaume's PR.
If so, I agree with both of you I think, let's move [1] to updateSignal and go whith that :)


[0] https://github.com/mozilla-b2g/gaia/pull/30501/files?w=1#diff-ea8d19ab311e1f3ec83ba5d1df0fccd0R114
[1] https://github.com/gmarty/gaia/blob/Bug-1171883-Signal-icon-is-not-animated-while-searching/apps/system/js/signal_icon.js#L84-86
Flags: needinfo?(etienne)
I updated the patch according to Etienne's comment.
Alberto, can I ask you to try? I can't reproduce this bug because my SIM card connects too quickly to the network.
(Assignee)

Comment 6

4 years ago
Created attachment 8623060 [details]
no-sginal-icon.png

I can't see any icon now, while searching
(Assignee)

Comment 7

4 years ago
Hey Guillaume, is probably going to be easier if I just steal this bug, because I'll be able to test it. Do you mind if I take this one with your WIP patch?

Thanks!
Flags: needinfo?(gmarty)
Not at all. Go for it!
Assignee: gmarty → apastor
Flags: needinfo?(gmarty)
(Assignee)

Comment 9

4 years ago
Created attachment 8624143 [details] [review]
Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/30646
Attachment #8620928 - Attachment is obsolete: true
Attachment #8620928 - Flags: review?(apastor)
Attachment #8624143 - Flags: review?(gmarty)
Comment on attachment 8624143 [details] [review]
Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/30646

It looks good. Thanks for taking over me.
Attachment #8624143 - Flags: review?(gmarty) → review+
(Assignee)

Comment 11

4 years ago
master: https://github.com/mozilla-b2g/gaia/commit/4d7da51b3923a0f54748c6619919e59c91058bb5
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.