Closed
Bug 1171883
Opened 9 years ago
Closed 9 years ago
[Statusbar] Signal icon is not animated while searching
Categories
(Firefox OS Graveyard :: Gaia::System::Status bar, Utility tray, Notification, defect)
Firefox OS Graveyard
Gaia::System::Status bar, Utility tray, Notification
ARM
Gonk (Firefox OS)
Tracking
(Not tracked)
RESOLVED
FIXED
2.2 S14 (12june)
People
(Reporter: apastor, Assigned: apastor)
References
Details
(Keywords: regression, Whiteboard: [systemsfe])
Attachments
(2 files, 1 obsolete file)
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•9 years ago
|
||
Regression of bug 1151502. :gmarty, could you please take a look?
Flags: needinfo?(gmarty)
Keywords: regression
Updated•9 years ago
|
Assignee: nobody → gmarty
Target Milestone: --- → 2.2 S14 (12june)
Comment 2•9 years ago
|
||
I reworked the logic for the searching state. What do you think, Alberto?
Flags: needinfo?(gmarty)
Attachment #8620928 -
Flags: review?(apastor)
Assignee | ||
Comment 3•9 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)
Comment 4•9 years ago
|
||
(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)
Comment 5•9 years ago
|
||
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•9 years ago
|
||
I can't see any icon now, while searching
Assignee | ||
Comment 7•9 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)
Assignee | ||
Comment 9•9 years ago
|
||
Attachment #8620928 -
Attachment is obsolete: true
Attachment #8620928 -
Flags: review?(apastor)
Attachment #8624143 -
Flags: review?(gmarty)
Comment 10•9 years ago
|
||
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•9 years ago
|
||
master: https://github.com/mozilla-b2g/gaia/commit/4d7da51b3923a0f54748c6619919e59c91058bb5
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•