Status

Firefox OS
Gaia::System::Lockscreen
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: gandalf, Assigned: gandalf)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
In anticipation for bug 992473 I'd like to try to get this in, since it's a separate cleanup patch.

The logic of lockscreen connstate code relies on textContent being empty instead of data-l10n-id being set.

There are also three states which are not localizable.

Lastly, the mock_l10n in unit tests does not set data-l10n-id at all.
(Assignee)

Updated

4 years ago
Assignee: nobody → gandalf
Status: NEW → ASSIGNED
(Assignee)

Comment 1

4 years ago
Created attachment 8433668 [details] [review]
pull request
(Assignee)

Comment 2

4 years ago
Comment on attachment 8433668 [details] [review]
pull request

Arthur, can you review this?

According to blame, you're the last person who touched this code, which makes you an owner of it in my eyes ;)

I'd like to land it separately from MutationObservers, but the code will work without any alternations after we land that.
Attachment #8433668 - Flags: review?(arthur.chen)
Comment on attachment 8433668 [details] [review]
pull request

f=me. But I would like the system app owner aware of the change.
Attachment #8433668 - Flags: review?(arthur.chen)
Attachment #8433668 - Flags: review?(alive)
Attachment #8433668 - Flags: feedback+
(Assignee)

Updated

4 years ago
Blocks: 992473
Comment on attachment 8433668 [details] [review]
pull request

I'd rather to have l10n guy aware.
Attachment #8433668 - Flags: review?(alive)
Attachment #8433668 - Flags: review+
Attachment #8433668 - Flags: feedback?(l10n)

Comment 5

4 years ago
Comment on attachment 8433668 [details] [review]
pull request

Not sure what localizers would do with operator or cell-broadcast?

Also, given that it's unclear, they should have a comment.

Frankly, I'd just drop both, not sure we're solving a problem there.
Attachment #8433668 - Flags: feedback?(l10n) → feedback-
(Assignee)

Comment 6

4 years ago
Hi Axel, thanks for your feedback. I don't know how I missed that I named the entity the same as a variable.

I agree we should not introduce non-localizable entities. I reworked my patch to cover directly injected textContent in order to avoid unnecessary l10n entities.

I'm re-requesting f and r.
(Assignee)

Updated

4 years ago
Attachment #8433668 - Flags: review?(alive)
Attachment #8433668 - Flags: review+
Attachment #8433668 - Flags: feedback?(l10n)
Attachment #8433668 - Flags: feedback-
Attachment #8433668 - Flags: feedback+
(Assignee)

Comment 7

4 years ago
by the way. Alive, while working on that, I noticed that LockScreenConnInfoManagerPrototype.updateConnStates is fired a lot.

 - it seems to be fired 6 times at phone boot before the screen shows up
 - it is fired 4 times when turning on airplane mode
 - it is fired 3 times when turning off airplane mode

Is that a bug or per design?
Flags: needinfo?(alive)

Comment 8

4 years ago
Comment on attachment 8433668 [details] [review]
pull request

f=me on the changes in the l10n file, and the absence of l10n wrappers that are no-ops in all locales.

I didn't really dive into the rest of the code.
Attachment #8433668 - Flags: feedback?(l10n) → feedback+
(In reply to Zibi Braniecki [:gandalf] from comment #7)
> by the way. Alive, while working on that, I noticed that
> LockScreenConnInfoManagerPrototype.updateConnStates is fired a lot.
> 
>  - it seems to be fired 6 times at phone boot before the screen shows up
>  - it is fired 4 times when turning on airplane mode
>  - it is fired 3 times when turning off airplane mode
> 
> Is that a bug or per design?

Transfer needinfo, arthur could confirm.
Flags: needinfo?(alive) → needinfo?(arthur.chen)
Attachment #8433668 - Flags: review?(alive) → review+
(Assignee)

Comment 10

4 years ago
Commit: https://github.com/mozilla-b2g/gaia/commit/bf988c38bf6d9316a1b95859ca88e30fb66005d3
Merge: https://github.com/mozilla-b2g/gaia/commit/536b3da91de6d864d162415433ffea316ed7618c

Thanks guys!

Leaving this bug open for ni=arthur
The connection states are determined by many attributes provided by gecko. The states should be updated when the events corresponding the attributes occur. When booting up or toggling airplane mode, gecko send those events, so that the state get updated frequently. This is neither a bug nor by design. However, we could improve it by making the requests of refresh queued for a short period of time to reduce the updating frequency.
Flags: needinfo?(arthur.chen)
(Assignee)

Comment 12

4 years ago
thanks!
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.