1517155 - Print preview is not diplayed anymore

Status: RESOLVED FIXED

(Calendar :: Printing, defect)

Description

[[:MakeMyDay]]

While looking at bug 1499475, I noticed that in the print preview dialog, the preview is not displayed anymore.

The problem is that calOutlookCSVImportExport.js is ANSI encoded both in c-c and c-esr60 (haven't checked c-b, but I assume it is the same), but the module loader at [1] expects the script to be UTF-8 encoded. This breaks any code in import-export subdirectory.

Comment 1

[[:MakeMyDay]]

Attachment: WrongEncodingOfCalOutlookCSVImportExport-V1.diff

This fixes the issue for me locally, though it's unformatunate that we have non-ascii characters in this file due to the hard coding of the csv headers.

Can you please cross check that it also works for you?

Is it possible to add a check task for the character encoding of the tree after landing patches siumilar to eslint? That might make some sense.

Comment 2

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

This fixes it for me as well. We most certainly can write a test for this, I have one in the works. Will r+ and upload when done.

Comment 3

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Fix - v2

Here we go. Maybe you can give the test a quick look.

Comment 4

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Joint try run here, hope it works: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=972146ac4632d9dbd08f14ad2b4b362cdc9ed66e

Comment 5

[[:MakeMyDay]]

The test failed (on automation as well as locally):

[task 2019-01-13T14:34:15.482Z] 14:34:15 INFO - xpcshell-icaljs.ini:comm/calendar/test/unit/test_import_csv.js | Starting test_file_encoding
[task 2019-01-13T14:34:15.483Z] 14:34:15 INFO - (xpcshell/head.js) | test test_file_encoding pending (2)
[task 2019-01-13T14:34:15.484Z] 14:34:15 INFO - (xpcshell/head.js) | test run_next_test 0 finished (2)
[task 2019-01-13T14:34:15.485Z] 14:34:15 INFO - Unexpected exception NS_ERROR_FILE_NOT_FOUND:
[task 2019-01-13T14:34:15.485Z] 14:34:15 INFO - promiseFileRead@/builds/worker/workspace/build/tests/xpcshell/tests/comm/calendar/test/unit/head_consts.js:275:22
[task 2019-01-13T14:34:15.486Z] 14:34:15 INFO - asynctest_file_encoding@/builds/worker/workspace/build/tests/xpcshell/tests/comm/calendar/test/unit/test_import_csv.js:89:24
[task 2019-01-13T14:34:15.487Z] 14:34:15 INFO - asyncrun_next_test/_run_next_test/<@/builds/worker/workspace/build/tests/xpcshell/head.js:1436:22
[task 2019-01-13T14:34:15.488Z] 14:34:15 INFO - async*_run_next_test@/builds/worker/workspace/build/tests/xpcshell/head.js:1436:10
[task 2019-01-13T14:34:15.489Z] 14:34:15 INFO - run@/builds/worker/workspace/build/tests/xpcshell/head.js:687:9
[task 2019-01-13T14:34:15.490Z] 14:34:15 INFO - _do_main@/builds/worker/workspace/build/tests/xpcshell/head.js:224:3
[task 2019-01-13T14:34:15.491Z] 14:34:15 INFO - _execute_test@/builds/worker/workspace/build/tests/xpcshell/head.js:528:5
[task 2019-01-13T14:34:15.491Z] 14:34:15 INFO - @-e:1:1
[task 2019-01-13T14:34:15.492Z] 14:34:15 INFO - exiting test

file.initWithPath(mozinfo.topsrcdir); seems to fail already - is mozinfo.topsrcdir definied?

Comment 6

[[:MakeMyDay]]

If you do another try push when you fixed the test, can you include the patch from bug 1503731, so we see whether your fix for bug 1513181 also makes that passing?

Comment 7

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

WFM locally, but in packaged tests it seems to be different. I'll do some more experimenting. If I don't get to that soon go ahead and push the fix and I'll take care of the test separately.

Comment 8

[[:MakeMyDay]]

Let's land this without the test (effectively v1 of the patch to repair the file encoding) as suggested above and add the test in a follow up bug to still get this into 60.5, since this bug prevents printing, importing and exporting of calendars.

For verifying that everything works as expectet, being able to see the preview in the calendar print dialog is sufficient.

Comment 9

[Jorg K (CEST = GMT+2)]

Comment on attachment 9036161 [details] [diff] [review]
Fix - v2

Apparently we want v1 to land.

Comment 10

[Jorg K (CEST = GMT+2)]

Comment on attachment 9033878 [details] [diff] [review]
WrongEncodingOfCalOutlookCSVImportExport-V1.diff

First version to change encoding from windows-1252 to UTF-8.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/fec3b0d58634
fix encoding of calOutlookCSVImportExport.js to be UTF-8 and not windows-1252. r=philipp

Comment 12

[Jorg K (CEST = GMT+2)]

TB 60.5 ESR / Cal 6.2.5:
https://hg.mozilla.org/releases/comm-esr60/rev/56ab5861f5cb37e01c1cd3aec27578ebe0509de2

Comment 13

[Jorg K (CEST = GMT+2)]

TB 65 beta 4, Cal 6.7:
https://hg.mozilla.org/releases/comm-beta/rev/c2bb7687c4ba56ceee22450a6080b367c52550ac

Comment 14

[[:MakeMyDay]]

Since the backport landed an we will not backport the test to 6.2.5, let's close this bug and move the test creation to bug 1523196.

Comment 15

[[:MakeMyDay]]

Comment on attachment 9036161 [details] [diff] [review]
Fix - v2

Removing this from my review queue since this bug is closed - please follow up in bug 1523196 and request again if you have an updated patch.