Last Comment Bug 637234 - timepicker element ignores locale specific configuration, e.g. 24h clocks
: timepicker element ignores locale specific configuration, e.g. 24h clocks
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: XP Toolkit/Widgets: XUL (show other bugs)
: 2.0 Branch
: All All
: -- normal (vote)
: ---
Assigned To: Neil Deakin
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-27 19:20 PST by George
Modified: 2012-04-18 12:46 PDT (History)
3 users (show)
enndeakin: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Suppose we should remove the readonly flag for now (2.33 KB, patch)
2011-05-30 03:45 PDT, Neil Deakin
neil: review+
Details | Diff | Review
fix test (2.42 KB, patch)
2011-05-30 08:36 PDT, Neil Deakin
maierman: review+
Details | Diff | Review

Description George 2011-02-27 19:20:32 PST
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] 2011-05-02 18:16:38 PDT
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 2011-05-30 03:45:09 PDT
Created attachment 536044 [details] [diff] [review]
Suppose we should remove the readonly flag for now
Comment 3 neil@parkwaycc.co.uk 2011-05-30 04:58:57 PDT
(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] 2011-05-30 08:28:25 PDT
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 2011-05-30 08:36:30 PDT
Created attachment 536101 [details] [diff] [review]
fix test

OK thanks! I guess I should be more careful with copying text.
Comment 7 Günter 2012-04-18 12:46:59 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.