637234 - timepicker element ignores locale specific configuration, e.g. 24h clocks

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[George]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b13pre) Gecko/20110227 Firefox/4.0b13pre
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b13pre) Gecko/20110227 Firefox/4.0b13pre

In Firefox 3, timepicker would use the time format (12 or 24 hour) as defined by OS long time. In Firefox 4, it always uses 12 hour time.

Reproducible: Always

Comment 1

[Nils Maier [:nmaier]]

The is24HourClock field is specified as readonly:
http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/datetimepicker.xml#249

But the field is only initialized in |_init|, which then fails:
http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/datetimepicker.xml#508

XBL seems to have implemented readonly fields in moz-2, hence the regression.
This regression is not limited to 24h clocks, but all locale specific runtime configuration (see workaround below).

CC'ing Neil, as he wrote the code it seems.

As a work-around, one might re-bind <timepicker> against the following binding that strips away the readonly stuff:
<binding id="timepicker" extends="chrome://global/content/bindings/datetimepicker.xml#timepicker">
  <implementation>
    <field name="is24HourClock">false</field>
    <field name="hourLeadingZero">false</field>
    <field name="minuteLeadingZero">true</field>
    <field name="secondLeadingZero">true</field>
    <field name="amIndicator">"AM"</field>
    <field name="pmIndicator">"PM"</field>
  </implementation>
</binding>

Comment 2

[Neil Deakin]

Attachment: Suppose we should remove the readonly flag for now

Comment 3

[[e-mail]]

(In reply to comment #1)
> XBL seems to have implemented readonly fields in moz-2, hence the regression.
That would have been bug 542406.

Comment 4

[Nils Maier [:nmaier]]

Comment on attachment 536044 [details] [diff] [review]
Suppose we should remove the readonly flag for now

Review of attachment 536044 [details] [diff] [review]:
-----------------------------------------------------------------

You didn't ask me for review, but r- from me anyway ;)

::: toolkit/content/tests/widgets/test_timepicker.xul
@@ +43,5 @@
>    tp.value = testtag_gettimestring(thour, tminute, tsecond);
>    testtag_comparetime(tp, testid + "set value", thour, tminute, tsecond);
>  
> +  var fdt = new Date(2000,0,1,16,7,9).toLocaleFormat("%X");
> +  is(tp.is24HourClock, Number(fdt) > 12, "is24HourClock");

This test is broken, and will fail on an is24HourClock machine, and only by accident succeed otherwise.
Number("16:07:09") is NaN

Comment 5

[Neil Deakin]

Attachment: fix test

OK thanks! I guess I should be more careful with copying text.

Comment 6

[Neil Deakin]

http://hg.mozilla.org/mozilla-central/rev/50103c9579b0

Comment 7

[Günter]

Not clear to me: Is there a fix?

Using TB 11.0.1 doesn't work with 
<timepicker id="timePickerCurrent" hideseconds="true" is24HourClock="false" />
to see 11:05pm instead of 23:05.

Comment 8

[Firefox Bug Husbandry Bot]

Moving to Core:XUL per https://bugzilla.mozilla.org/show_bug.cgi?id=1455336