Closed Bug 1734792 Opened 1 year ago Closed 1 year ago

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

Categories

(MailNews Core :: Composition, defect, P5)

Tracking

(thunderbird_esr91 unaffected, thunderbird94 unaffected)

RESOLVED FIXED
95 Branch
Tracking Status
thunderbird_esr91 --- unaffected
thunderbird94 --- unaffected

People

(Reporter: intermittent-bug-filer, Assigned: TYLin)

References

(Regression)

Details

(Keywords: intermittent-failure, regression)

Attachments

(2 files)

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.)

Flags: needinfo?(aethanyc)

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.

Flags: needinfo?(aethanyc) → needinfo?(geoff)

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.

Assignee: nobody → aethanyc
Flags: needinfo?(geoff)
Summary: Intermittent comm/mailnews/compose/test/unit/test_longLines.js | testBodyWithLongLine - [testBodyWithLongLine : 55] → Perma fail comm/mailnews/compose/test/unit/test_longLines.js | testBodyWithLongLine - [testBodyWithLongLine : 55]

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

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

Depends on D128002

Pushed by aethanyc@gmail.com:
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
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 95 Branch
You need to log in before you can comment on or make changes to this bug.