1060333 - [Dialer] Replace span with hidden mechanism in CallScreen.js in showClock

Status: RESOLVED FIXED

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

Description

[Tamara Hills [:thills]]

In the showClock function, a <span> is used to hide the AM/PM from the time that shows up when a user receives an incoming call.  We should use a hidden mechanism here.

Comment 1

[Doug Sherk (:drs) (inactive)]

After thinking about this some more, I don't think we really need to do anything here other than comment what's happening better. Adding a .hidden class would be disingenuous anyways since the span is shown in the mozHour12=true case. Thanks for filing this anyways.

Comment 2

[Doug Sherk (:drs) (inactive)]

Reopened as I was wrong about the visibility of this element depending on the mozHour12 setting.

This also lacks decent test coverage, so we should fix that too. There's one test that covers it indirectly.

Comment 3

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PULL REQUEST] Remove the use of innerHTML for displaying the time in the lockscreen r=drs,gweng

This removes the use of the hidden span to hide the AM/PM component of the clock. Instead it strips the unwanted part of the string via a regular expression. I've also removed the now redundant CSS selector. I've done the same change in the lockscreen since the code is essentially the same hence the double review (Doug for the callscreen and Greg for the lockscreen).

Note that I've written the patch on top of bug 1176193 and since that's not merged yet the patches appear in the PR but there's no interactions between the two save for the eslint xfail.list.

Comment 4

[Greg Weng [:snowmantw][:gweng][:λ]]

Comment on attachment 8625194 [details] [review]
[PULL REQUEST] Remove the use of innerHTML for displaying the time in the lockscreen r=drs,gweng

I think somethings need to be elaborated more, so I left some comments on the PR.  Thanks.

Comment 5

[Gabriele Svelto [:gsvelto] (PTO)]

Comment on attachment 8625194 [details] [review]
[PULL REQUEST] Remove the use of innerHTML for displaying the time in the lockscreen r=drs,gweng

I've updated the comment regarding the AM/PM component of the clock on the lockscreen. Hopefully this should make the intent clearer.

Comment 6

[Greg Weng [:snowmantw][:gweng][:λ]]

Comment on attachment 8625194 [details] [review]
[PULL REQUEST] Remove the use of innerHTML for displaying the time in the lockscreen r=drs,gweng

The part now looks good. Thanks!

Comment 7

[Doug Sherk (:drs) (inactive)]

Comment on attachment 8625194 [details] [review]
[PULL REQUEST] Remove the use of innerHTML for displaying the time in the lockscreen r=drs,gweng

It would be nice if we could think of a better method to strip away the AM/PM tag, but I can't.

Comment 8

[Gabriele Svelto [:gsvelto] (PTO)]

I think this is one of the use cases we should be able to clean up with the new l10n libraries, the date support isn't there yet though AFAIK.

Comment 9

[Gabriele Svelto [:gsvelto] (PTO)]

Merged to gaia/master 8c65fa5e3879ecb528206494998ce0e58c1f77a9

https://github.com/mozilla-b2g/gaia/commit/8c65fa5e3879ecb528206494998ce0e58c1f77a9