646561 - "ASSERTION: Invalid offset" with soft hyphen in table, bidi

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Jesse Ruderman]

Attachment: testcase

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

First seen at 4am today. I bet it's a regression from bug 418975.

Comment 1

[Jesse Ruderman]

Attachment: stack trace

Comment 2

[Jonathan Kew [:jfkthame]]

(In reply to comment #0)
> Created attachment 523061 [details]
> testcase
> 
> ASSERTION: Invalid offset: 'aOffset <= mSkipChars->mCharCount', file
> gfx/thebes/gfxSkipChars.cpp, line 92
> 
> First seen at 4am today. I bet it's a regression from bug 418975.

Argh - yes, I expect it is. I'll take a look.... if it's not easy to fix, we may have have to back that out.

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: patch, use GetInFlowContentLength() rather than frag->GetLength() as the basis for length of text the PropertyProvider covers

In the mixed-direction case, the text represented by the textRun (up to flowEndInTextRun) may not correspond to the whole of the nsTextFragment, as assumed by the patch from bug 418975. Instead I think we should be using GetInFlowContentLength() to find the relevant length for the PropertyProvider.

(We should also add this testcase as a crashtest.)

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 523122 [details] [diff] [review]
patch, use GetInFlowContentLength() rather than frag->GetLength() as the basis for length of text the PropertyProvider covers

Yes, definitely add the test.

Comment 5

[Jonathan Kew [:jfkthame]]

Pushed to cedar, as m-c lacks a sheriff:
http://hg.mozilla.org/projects/cedar/rev/af3ed0b17ec7 (patch)
http://hg.mozilla.org/projects/cedar/rev/fbffbfe69e5d (crashtest)

Comment 6

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/af3ed0b17ec7
http://hg.mozilla.org/mozilla-central/rev/fbffbfe69e5d