1734792 - Perma fail comm/mailnews/compose/test/unit/test_longLines.js | testBodyWithLongLine - [testBodyWithLongLine : 55]

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect, P5)

Description

[Treeherder Bug Filer]

Filed by: geoff [at] darktrojan.net
Parsed log: https://treeherder.mozilla.org/logviewer?job_id=354106891&repo=comm-central
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/CTzOZMwERbu8ouf1RhAang/runs/0/artifacts/public/logs/live_backing.log

---

I'm not 100% sure this is bug 1733876, but it's looking pretty suspect.

Comment 1

[Geoff Lankow (:darktrojan)]

Our test is failing in at least these three places, the space and one of the \r\ns is missing.

(I don't know anything about this test, I'm just reporting the failure. Somebody in #maildev may be able to help further if you need it.)

Comment 2

[Ting-Yu Lin [:TYLin] (UTC-8)]

Is this a perma fail or an intermittent? I could not see the test failed in the previous treeherder job https://treeherder.mozilla.org/jobs?repo=comm-central&revision=0cf0d7c55bcec3af55266ff9b2fc6615b5547811

Bug 1733876 is likely to be the cause, and I don't expect the bug to change the behavior. However, gecko's test coverage to the plain text serializer is not great, I'd love to dig into more in some edge cases.

Comment 3

[Ting-Yu Lin [:TYLin] (UTC-8)]

This is perma fail. I can reproduce this locally in latest mozilla-central via running ./mach test comm/mailnews/compose/test/unit/test_longLines.js.

Reverting bug 1733876 Part 3, 4, 5 fixed the testcase.

Comment 4

[Ting-Yu Lin [:TYLin] (UTC-8)]

Attachment: Bug 1734792 Part 1 - Fix the logic where we break only at ascii spaces in FindWrapIndexForContent(). r?m_kato

Bug 1733876 Part 3 wrongly changed the behavior because it failed to
consider that goodSpace may equals to -1 in the following forward search
while loop, which coincidentally is NS_LINEBREAKER_NEED_MORE_TEXT.

    while (goodSpace >= 0 && !nsCRT::IsAsciiSpace(mContent.CharAt(goodSpace))) {
      goodSpace--;
    }

Thus, when goodSpace == -1, we need to run the original forward search logic here.
https://searchfox.org/mozilla-central/rev/afe1ba25ee057a6118b5b990f994bdf9eb5ffb43/dom/serializers/nsPlainTextSerializer.cpp#180-185

The gtest is adapted from
https://searchfox.org/comm-central/rev/c77102402f9ddb2742f6782aa5b73362be39d5f5/mailnews/compose/test/unit/test_longLines.js#134-145

Comment 5

[Ting-Yu Lin [:TYLin] (UTC-8)]

Attachment: Bug 1734792 Part 2 - Remove NS_LINEBREAKER_NEED_MORE_TEXT from the logic where break only at ascii spaces. r?m_kato

The logic has nothing to do with LineBreaker, and it shouldn't rely on
NS_LINEBREAKER_NEED_MORE_TEXT == -1.

Depends on D128002

Comment 6

[Intermittent Failures Robot]

1 failures in 3626 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• comm-central: 1

Platform and build breakdown:

• linux64: 1
  • opt: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1734792&startday=2021-10-04&endday=2021-10-10&tree=all

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aaedd1e3ae0c
Part 1 - Fix the logic where we break only at ascii spaces in FindWrapIndexForContent(). r=m_kato
https://hg.mozilla.org/integration/autoland/rev/08d68749d217
Part 2 - Remove NS_LINEBREAKER_NEED_MORE_TEXT from the logic where break only at ascii spaces. r=m_kato

Comment 8

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/aaedd1e3ae0c
https://hg.mozilla.org/mozilla-central/rev/08d68749d217