[DateTimeInput] (l10n) 12/24hr format for <input type=time> based on locale

RESOLVED FIXED in Firefox 55

Status

()

Core
Layout: Form Controls
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: jessica, Assigned: jessica)

Tracking

(Blocks: 1 bug)

Trunk
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Comment 1

a year ago
Hi zibi, do you know which API should we use to know whether a locale uses 12 or 24hr clock? Thanks.
Flags: needinfo?(gandalf)
Yep, use `(new Intl.DateTimeFormat('pl', {hour: 'numeric'})).resolvedOptions().hour12`
Flags: needinfo?(gandalf)
(Assignee)

Comment 3

a year ago
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #2)
> Yep, use `(new Intl.DateTimeFormat('pl', {hour:
> 'numeric'})).resolvedOptions().hour12`

Cool! Thanks for the prompt reply.
(Assignee)

Updated

a year ago
Assignee: nobody → jjong
Comment hidden (mozreview-request)

Comment 5

a year ago
mozreview-review
Comment on attachment 8847486 [details]
Bug 1347069 - [DateTimeInput] (l10n) 12/24hr format for <input type=time> based on locale.

https://reviewboard.mozilla.org/r/120470/#review123010

Stealing mconley's review.

Looks good but I'd like to see this again before landing.

::: toolkit/content/widgets/datetimebox.xml:672
(Diff revision 1)
>            return { amString, pmString };
>          ]]>
>          </body>
>        </method>
>  
> +      <method name="getHourFormatForLocale">

The method name here implies it returns some kind of format information when really it just returns true or false. Let's name it "is12HourTime" unless you have other suggestions?

::: toolkit/content/widgets/datetimebox.xml:676
(Diff revision 1)
>  
> +      <method name="getHourFormatForLocale">
> +        <parameter name="aLocales"/>
> +          <body>
> +          <![CDATA[
> +            this.log("getHourFormatForLocale: " + aLocales);

Remove the logging before landing please
Attachment #8847486 - Flags: review-
(Assignee)

Comment 6

a year ago
mozreview-review-reply
Comment on attachment 8847486 [details]
Bug 1347069 - [DateTimeInput] (l10n) 12/24hr format for <input type=time> based on locale.

https://reviewboard.mozilla.org/r/120470/#review123010

> The method name here implies it returns some kind of format information when really it just returns true or false. Let's name it "is12HourTime" unless you have other suggestions?

`is12HourTime` is good, thanks.

> Remove the logging before landing please

I have a debug flag, so the logging is not enabled by default. Do I still need to remove it?
Clearing review flags, as Mossop has graciously stepped in to review this stuff (not only to help clear my review queue, but also to spread the knowledge of how the DateTime stuff works around!).
(Assignee)

Comment 8

a year ago
Thanks to Mike and Mossop! And ni? for comment 6.
Flags: needinfo?(dtownsend)

Comment 9

a year ago
mozreview-review-reply
Comment on attachment 8847486 [details]
Bug 1347069 - [DateTimeInput] (l10n) 12/24hr format for <input type=time> based on locale.

https://reviewboard.mozilla.org/r/120470/#review123010

> I have a debug flag, so the logging is not enabled by default. Do I still need to remove it?

That's fine then, leave it in if you like.
Flags: needinfo?(dtownsend)
Comment hidden (mozreview-request)

Comment 11

a year ago
mozreview-review
Comment on attachment 8847486 [details]
Bug 1347069 - [DateTimeInput] (l10n) 12/24hr format for <input type=time> based on locale.

https://reviewboard.mozilla.org/r/120470/#review124496

Looks good, thanks!
Attachment #8847486 - Flags: review?(dtownsend) → review+

Comment 13

a year ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/c8e3f323a687
[DateTimeInput] (l10n) 12/24hr format for <input type=time> based on locale. r=mossop
Keywords: checkin-needed

Comment 14

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c8e3f323a687
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.