Closed Bug 847242 Opened 7 years ago Closed 6 years ago

"ASSERTION: Invalid offset" and hang with bidi-override, :first-letter

Categories

(Core :: Layout, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: jruderman, Assigned: smontagu)

References

Details

(Keywords: assertion, hang, testcase, Whiteboard: [fuzzblocker])

Attachments

(3 files, 1 obsolete file)

###!!! ASSERTION: Invalid offset: 'aOffset <= mSkipChars->mCharCount', file gfx/thebes/gfxSkipChars.cpp, line 61

###!!! ASSERTION: Text run does not map enough text for our reflow: 'gfxSkipCharsIterator(iter).ConvertOriginalToSkipped(offset + length) <= mTextRun->GetLength()', file layout/generic/nsTextFrameThebes.cpp, line 7870

###!!! ASSERTION: redo line on totally empty line with non-empty band...: 'aFloatAvailableSpace.mHasFloats', file layout/generic/nsBlockFrame.cpp, line 3606
Summary: "ASSERTION: Invalid offset an → "ASSERTION: Invalid offset" and hang with bidi-override, :first-letter
Attached file stacks
Due to the "Invalid offset" assertion being in my ignore list, I missed a security bug (bug 970710) for several months :(
Whiteboard: [fuzzblocker]
Attached patch 847242.diff (obsolete) — Splinter Review
I'm not quite sure why the hang only happens with the combination of bidi-override and first-letter, but the underlying bug seems to be that the U+2029 PARAGRAPH SEPARATOR makes the Bidi engine end the paragraph, but is just treated as whitespace elsewhere in the code.
Attachment #8384478 - Flags: review?(roc)
Comment on attachment 8384478 [details] [diff] [review]
847242.diff

Review of attachment 8384478 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/nsBidiPresUtils.cpp
@@ +644,5 @@
>    if (aBpd->BufferLength() < 1) {
>      return NS_OK;
>    }
> +  for (uint32_t i = 0; i < ArrayLength(kSeparators); ++i) {
> +    aBpd->mBuffer.ReplaceChar(kSeparators[i], kSpace);

This doesn't seem efficient. Let's make ReplaceChar work with a UTF16 array instead, or you prefer, just make two ReplaceChar calls, one with a char string and another with 0x2029.

::: layout/reftests/bidi/847242-1.html
@@ +3,5 @@
> + <head>
> +   <meta charset="utf-8">
> +   <style>
> +    div {
> +     unicode-bidi: bidi-override; 

trailing whitespace
Attachment #8384478 - Flags: review?(roc) → review-
Depends on: 979556
Attached patch Updated patchSplinter Review
Attachment #8384478 - Attachment is obsolete: true
Attachment #8385662 - Flags: review?(roc)
https://hg.mozilla.org/mozilla-central/rev/b9f4ce275fdd
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.