1404581 - TEST-UNEXPECTED-FAIL | tests/mozmill/composition/test-drafts.js | test-drafts.js::test_remove_space_stuffing_format_flowed

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Jorg K (CEST = GMT+2)]

First seen: Sat Sep 30, 2017, 1:48:57

M-C last good: cd9c8c48e4b3ded47a776f757008f3dcf5
M-C first bad: 57f68296c350469d73d788eb3695a89894

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cd9c8c48e4b3ded47a776f757008f3dcf5&tochange=57f68296c350469d73d788eb3695a89894

Could be related to bug Bug 1393140 or bug 1404106 since there are editor changes in those bugs.

On the various platforms there are different sets of failing tests.

Linux 32:
1) composition/test-drafts.js | test-drafts.js::test_remove_space_stuffing_format_flowed

Linux 64:
1) composition/test-drafts.js | test-drafts.js::test_remove_space_stuffing_format_flowed
2) composition/test-image-insertion-dialog.js | test-image-insertion-dialog.js::test_image_insertion_dialog_persist
   (regular failure??) - bug 1246094
3) composition/test-send-format.js | test-send-format.js::test_msg_convertibility 
4) composition/test-image-display.js | test-image-display.js::test_cid_image_compose
5) content-tabs/test-about-support.js | test-about-support.js::test_send_via_email_public
   (regular failure??) - bug 1373809
6) content-tabs/test-about-support.js | test-about-support.js::test_send_via_email_private
   (regular failure??) - bug 1373809

Windows:
1) composition\test-drafts.js | test-drafts.js::test_remove_space_stuffing_format_flowed
2) composition\test-forwarded-content.js | test-forwarded-content.js::test_forwarded_subj 
3) composition\test-image-display.js | test-image-display.js::test_cid_image_compose
4) composition\test-image-insertion-dialog.js | test-image-insertion-dialog.js::test_image_insertion_dialog_persist
5) composition\test-multipart-related.js | test-multipart-related.js::test_basic_multipart_related 
6) composition\test-send-format.js | test-send-format.js::test_msg_convertibility

I'll have to run those tests locally to see which one really fail and which failures are a consequence of earlier failing tests.

Aceman, a lot of work here.

Comment 1

[Jorg K (CEST = GMT+2)]

OK, trying to see how much is really broken:

1) mozmake SOLO_TEST=composition/test-drafts.js mozmill-one fails with
test_remove_space_stuffing_format_flowed
  EXCEPTION: Something went wrong with space stuffing
    at: test-folder-display-helpers.js line 2918
       assert_true test-folder-display-helpers.js:2918 11
       test_remove_space_stuffing_format_flowed test-drafts.js:268 5

I can see that instead of having three lines
NoSpace
 OneSpace
  TwoSpaces
I only see two lines
NoSpace OneSpace
  TwoSpaces
when the draft is edited again. I guess that can be reproduced manually.
Looks like something went bust in Core::Editor.

L2)W4) composition/test-image-insertion-dialog.js passes.
L3)W6) composition/test-send-format.js passes.
L4)W3) composition/test-image-display.js passes.
L5) and L6) are normal failures.
W2) composition/test-forwarded-content.js passes.
W5) composition/test-multipart-related.js passes.

So it's really only composition/test-drafts.js that fails and drags the others down :-(

I'll disable this for now until we have time to investigate this further.

Comment 2

[Jorg K (CEST = GMT+2)]

Attachment: 1404581-temp-disable.patch

Comment 3

[Jorg K (CEST = GMT+2)]

Basically there is nothing we can do here in TB. When we get the content of the draft and hand it to the editor, something goes wrong.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/007e4cf39ff4
temporarily disable test-drafts.js::test_remove_space_stuffing_format_flowed. rs=bustage-fix

Comment 5

[Jorg K (CEST = GMT+2)]

You can see the problem locally. Prepare a plaintext draft with
NoSpace
 OneSpace
  TwoSpaces
then edit the draft. There first two lines are glued together.

Comment 6

[(no longer active)]

Attachment: Fallback to the slow path if we're in Thunderbird's plaintext editor

Jorg, do you mind testing this patch to see whether it fixes the test failures (and also whether it unbreaks Thunderbird when actually using it)?  Thanks!

Comment 7

[Jorg K (CEST = GMT+2)]

Comment on attachment 8914432 [details] [diff] [review]
Fallback to the slow path if we're in Thunderbird's plaintext editor

Thank you Ehsan, this works fine with manual testing and in the automated test.

Comment 8

[(no longer active)]

Note for the review, this just changes the conditional for the last child to check for AsHTMLEditor() and moves the two sides of the if branch, so there isn't as much code change in the patch as it may appear from a first glance.  :-)

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d57ba091571a
Fallback to the slow path if we're in Thunderbird's plaintext editor; r=masayuki

Comment 10

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/mozilla-central/rev/d57ba091571a

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/9c2aabee10e9
Backed out changeset 007e4cf39ff4: re-enable temporarily disabled test. a=jorgk

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d57ba091571a

Comment 13

[Onno Ekker [:nONoNonO UTC+1]]

Comment on attachment 8913949 [details] [diff] [review]
1404581-temp-disable.patch

Patch was backed out with #c11