1325792 - Port bug 1301640 to calendar

Status: RESOLVED FIXED

(Calendar :: General, defect)

Description

[Jorg K (CEST = GMT+2)]

+++ This bug was initially created as a clone of Bug #1325745 +++

Xpcshell failures here on Linux 32 bit, most likely because the date/time format changed due to bug 1301640:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=a6298203de72b408d580f604e9a336d2b70068bf&selectedJob=55641

TEST-UNEXPECTED-FAIL | xpcshell-icaljs.ini:calendar/test/unit/test_ltninvitationutils.js | compareInvitationOverlay_test - [compareInvitationOverlay_test : 537] (test #3): imipHtml-when-content - false == true

In the test we read:
// time format is platform dependent, so we use alternative result sets here
// the first three are to meet configurations running for automated tests, the
// following are some flavours that may occur - if you get a failure for this test,
// add your pattern here

Comment 1

[Jorg K (CEST = GMT+2)]

Note that bug 1301640 has been backed out for now:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a6882a49e907bd01ab7b782a0985614f4565e02e

Comment 2

[Jorg K (CEST = GMT+2)]

Bug 1301640 and bug 1325745 have relanded. So you need to fix this now.

Comment 3

[[:MakeMyDay]]

Thanks for the heads-up. Test failures on OSX and Linux32 only.

Comment 4

[[:MakeMyDay]]

There seems to more wrong than just the datetime formatting. Currently the test expects for Linux and Mac

<span xmlns="" class="added">Wed 9 Sep 2015 02:00 PM – 03:00 PM</span><br xmlns=""/><span xmlns="" class="removed">Wed 9 Sep 2015 01:00 PM – 02:00 PM</span>"

and 

<span xmlns="" class="added">Wed 9 Sep 2015 2:00 PM – 3:00 PM</span><br xmlns=""/>"<span xmlns="" class="removed">Wed 9 Sep 2015 1:00 PM – 2:00 PM</span>

while dumping the result shows in [1]

<span xmlns="" class="added">Wed 9 Sep 2015 4:08 PM – 3:57 PM</span><br xmlns=""/><span xmlns="" class="removed">Wed 9 Sep 2015 4:20 PM – 4:08 PM</span>

and

<span xmlns="" class="added">Wed 9 Sep 2015 4:08 PM – 3:57 PM</span><br xmlns=""/><span xmlns="" class="removed">Wed 9 Sep 2015 4:20 PM – 4:08 PM</span>

So apart from reformatting, something is wrong with the time itself. That doesn't look like a timezone problem but how we deal with the time in our dateTimeFormatter.

And interestingly, the test passes on Linux64, which I would have expected to fail for the formatting, too, as the date formatting should have been OS but not architecture specific.

[1] https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=94566c4f4b06d1953159f300e15416d2abd9b7cd

Comment 5

[[:MakeMyDay]]

Ok, let's see whether the latest push for bug 1325745 changes the obscure times for Linux32 and Mac:

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=8d851d149b0a588d9ebfed014f6ab806fd5e830a

Comment 6

[[:MakeMyDay]]

The obscure time handling is still present (at least on Linux32):

10:19:56     INFO -  PROCESS | 23522 | <span xmlns="" class="added">Wed 9 Sep 2015 4:08 PM – 3:57 PM</span><br xmlns=""/><span xmlns="" class="removed">Wed 9 Sep 2015 4:20 PM – 4:08 PM</span>

Comment 7

[[:MakeMyDay]]

Philipp, do you observe other datetime problems then with the invitation preview on OSX with a current Daily?

Comment 8

[[:MakeMyDay]]

Datepicker on Linux32:

https://bug1329957.bmoattachments.org/attachment.cgi?id=8825470

Comment 9

[Markus Adrario [:Taraman]]

Attachment: Linux32 Time-band.png

First I did not get any error messages.
After looking at the date-time-picker with DOM-Inspector, I found these two in the error-console:
>Use of attributes’ nodeValue attribute is deprecated. Use value instead.  lightning-item-iframe.xul
>Use of getAttributeNode() is deprecated. Use getAttribute() instead.  textbox.xml:214:10

I don't know at which point they appeared and could not reproduce them.

Also the times in the Day-View are borked, as can be seen in the Screenshot.

In DOM-Inspector, I found that the xul:datepicker's mValue has the correct date and time set, but the HTML-Input field has 1/1/1970 as value, which is the same as it's defaultValue.

Comment 10

[Jorg K (CEST = GMT+2)]

After landing of bug 1329494, Xpcshell tests are a little greener and these calendar failures are easier to see.

Can someone explain to me why test_ltninvitationutils.js passes on Mac OS X debug but not opt?

Comment 11

[Jorg K (CEST = GMT+2)]

I think you need to dig into the ICU library (intl/locale/DateTimeFormatICU.cpp) and see what happens. Maybe it's buggy.

Comment 12

[Markus Adrario [:Taraman]]

I traced it down to the timeFormatter.
The following is console output from a linux32 System:

> formatter.FormatTime("", Components.interfaces.nsIScriptableDateFormat.timeFormatNoSeconds, 0, 0, 0);
> "00:37"
> formatter.FormatTime("", Components.interfaces.nsIScriptableDateFormat.timeFormatNoSeconds, 0, 9, 0);
> "00:46"
> formatter.FormatTime("", Components.interfaces.nsIScriptableDateFormat.timeFormatNoSeconds, 0, 23, 0);
> "01:00"
> formatter.FormatTime("", Components.interfaces.nsIScriptableDateFormat.timeFormatNoSeconds, 1, 0, 0);
> "00:25"
> formatter.FormatTime("", Components.interfaces.nsIScriptableDateFormat.timeFormatNoSeconds, 1, 23, 0);
> "00:48"
> formatter.FormatTime("", Components.interfaces.nsIScriptableDateFormat.timeFormatNoSeconds, 9, 0, 0);
> "01:16"

Comment 13

[Markus Adrario [:Taraman]]

https://dxr.mozilla.org/comm-central/rev/ac3275723df59db0f09198fdb61b51e7002c391a/mozilla/intl/locale/nsScriptableDateFormat.cpp#58

Unfortunately my C++ is not very good...

Comment 14

[Stefan Sitter [:ssitter]]

(In reply to Markus Adrario [:Taraman] from comment #12)

Are you able to reproduce the problem in an official Firefox nightly build? If yes this might be worth reporting as Core problem?

Comment 15

[alta88]

Sure enough, running FormatTime on a Fx 32bit nightly on linux32 boot led to the wrong time. This is definitely a Core bug. The same command in browser console returned the expected "09:00 AM" on Fx64/linux64.

Cc["@mozilla.org/intl/scriptabledateformat;1"].getService(Ci.nsIScriptableDateFormat).FormatTime("", 2, 9, 0, 0);

Seems like time for js to go to Intl.DateTimeFormat, the bugs there (option of 'hour': '2-digit' doesn't return leading 0 as expected) have a better chance though than xpcom interfaces; m-c seems fairly determined to go ICU.

Comment 16

[Markus Adrario [:Taraman]]

(In reply to Stefan Sitter from comment #14)

Same in the Browser (53.0a1 (2017-01-15) (32-bit):
>Cc["@mozilla.org/intl/scriptabledateformat;1"].getService(Ci.nsIScriptableDateFormat).FormatTime("", 2, 9, 0, 0);
>"01:16"

Comment 17

[André Bargull [:anba]]

(In reply to alta88 from comment #15)
> Seems like time for js to go to Intl.DateTimeFormat, the bugs there (option
> of 'hour': '2-digit' doesn't return leading 0 as expected) have a better
> chance though than xpcom interfaces;

This sounds like bug 1284868.

Comment 18

[Markus Adrario [:Taraman]]

As of this comment [1], the scriptableDateFormat is deprecated. So I fear, we can't expect a fix there - is this correct?
If so, we should consider switching to the new interface Intl.DateTimeFormat.

I could try to prepare that if desired.


[1]: https://bugzilla.mozilla.org/show_bug.cgi?id=421790#c3

Comment 19

[Martin Schröder [:mschroeder]]

Markus, see bug 1229684 for the switch to Intl.DateTimeFormat.

Comment 20

[Jorg K (CEST = GMT+2)]

(In reply to André Bargull from comment #17)
> This sounds like bug 1284868.
Would someone care to explain to me why bug 1284868 of July 2016 is relevant here since the regressing bug 1301640 landed in Jan 2017.

Comment 21

[[:MakeMyDay]]

It seems obvious there is still an uncovered bug in the recent implemenation of DateTime formatting in core which breaks Linux 32 (although the code there looks ok to me, but I'm not an expert in cpp). 

But the way forward here is switching to use Intl.DateTimeFormat instead of nsScriptableDateFormat in calendar as the latter is deprecated anyway. There may be still limitations like reported in bug 1284868 we would need to work around to gain formatting capabilities acceptable for calendar client. But we can discuss that in the other bug.

Markus, can you give bug 1229684 a try? My time is quite limited atm. I would leave this bug open for now to fix remaining failures in our tests in case the core issue is uncovered and fixed before landing bug 1229684.

Comment 22

[Markus Adrario [:Taraman]]

I could have a look at bug 1229684, but I'm not sure if this is really our problem.
There it seems, that only the seconds-display is affected which should be no factor for us here.

First thing we should have bug 1229684 fixed and then see to the rest.

Comment 23

[André Bargull [:anba]]

(In reply to Jorg K (GMT+1) from comment #20)
> (In reply to André Bargull from comment #17)
> > This sounds like bug 1284868.
> Would someone care to explain to me why bug 1284868 of July 2016 is relevant
> here since the regressing bug 1301640 landed in Jan 2017.

This was in response to the Intl.DateTimeFormat bits in comment #15.

Comment 24

[Jorg K (CEST = GMT+2)]

With bug 1229684 being fixed, I think we're done here.