Print preview is not diplayed anymore

RESOLVED FIXED in 6.2.5

Status

RESOLVED FIXED
3 months ago
a month ago

People

(Reporter: MakeMyDay, Assigned: MakeMyDay)

Tracking

(Blocks: 1 bug, {regression})

Lightning 6.2.4
6.2.5
regression

Details

Attachments

(2 attachments)

(Assignee)

Description

3 months ago
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.
(Assignee)

Comment 1

3 months ago
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.
Assignee: nobody → makemyday
Status: NEW → ASSIGNED
Attachment #9033878 - Flags: review?(philipp)
Attachment #9033878 - Flags: approval-calendar-esr?(philipp)
Attachment #9033878 - Flags: approval-calendar-beta?(philipp)

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.

Posted patch Fix - v2 β€” β€” Splinter Review

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

Attachment #9033878 - Attachment is obsolete: true
Attachment #9033878 - Flags: review?(philipp)
Attachment #9033878 - Flags: approval-calendar-esr?(philipp)
Attachment #9033878 - Flags: approval-calendar-beta?(philipp)
Attachment #9036161 - Flags: review?(makemyday)
Attachment #9036161 - Flags: approval-calendar-esr+
Attachment #9036161 - Flags: approval-calendar-beta+
(Assignee)

Comment 5

2 months ago

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 - async
run_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?

(Assignee)

Comment 6

2 months ago

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?

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.

(Assignee)

Comment 8

2 months ago

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.

Keywords: checkin-needed

Comment 9

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

Apparently we want v1 to land.
Attachment #9036161 - Flags: approval-calendar-esr+
Attachment #9036161 - Flags: approval-calendar-beta+

Updated

2 months ago
Keywords: leave-open

Comment 10

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

First version to change encoding from windows-1252 to UTF-8.
Attachment #9033878 - Flags: review+
Attachment #9033878 - Flags: approval-calendar-esr+
Attachment #9033878 - Flags: approval-calendar-beta+

Updated

2 months ago
Attachment #9033878 - Attachment is obsolete: false

Comment 11

2 months ago

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

Keywords: checkin-needed

Updated

2 months ago
Target Milestone: --- → 6.8

Comment 12

2 months ago
Target Milestone: 6.8 → 6.2.5
(Assignee)

Updated

2 months ago
Blocks: 1523196
(Assignee)

Comment 14

2 months ago

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.

Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
Resolution: --- → FIXED
(Assignee)

Updated

2 months ago
Keywords: leave-open
(Assignee)

Comment 15

a month ago
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.
Attachment #9036161 - Flags: review?(makemyday)
You need to log in before you can comment on or make changes to this bug.