1221904 - "ASSERTION: Invalid offset" with bidi text change

Status: RESOLVED WORKSFORME

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

Description

[Jesse Ruderman]

Attachment: testcase

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

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

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Daniel Veditz [:dveditz]]

It's possible the code after the asserts handles this condition in which case the security severity I'm assigning is overstated. Please let us know after this is analyzed.

Comment 3

[Mats Palmgren (inactive)]

The testcase WFM in trunk, but I can reproduce it in Aurora.
Bisecting when it was first fixed gives this rev:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a675fc80caa9
i.e. bug 1216096, which makes sense looking at the changes.

Maybe there *is* a bug here though and that the backout in bug 1216096 merely
wallpapers it and that it will come back and bite us again? I don't know.

Seems like we should uplift bug 1216096 to the affected branches though.

[Tracking Requested - why for this release]:
sec-high (possibly)

Comment 4

[Simon Montagu :smontagu]

(In reply to Mats Palmgren (:mats) from comment #3)
> Maybe there *is* a bug here though and that the backout in bug 1216096 merely
> wallpapers it and that it will come back and bite us again? I don't know.

I can't swear to it, but I think it's more likely that bug 1164963 caused the bug and the backout fixed it.

> 
> Seems like we should uplift bug 1216096 to the affected branches though.

See bug 1216096 comment 11. I would very much have liked to uplift bug 1216096, but that won't be possible unless we can fix the regression on Aurora.

Comment 5

[Mats Palmgren (inactive)]

OK, thanks.

"Bug 1164963 - Hi-res search icons for localized search plugins"
doesn't seem like the right bug though.  It doesn't have any patches
so it seems unlikely to have caused the regression.

Resolving as WFM for mozilla45 -- fixed by the backout in bug 1216096.
I'm guesssing 43/44 are wontfix based on comment 4, but I'll leave
that for others to decide.

Comment 6

[Mats Palmgren (inactive)]

Bug 1164693 is probably the right bug number for the bug that originally regressed it.

Comment 7

[Simon Montagu :smontagu]

Should be fixed43 and fixed44 by the uplift of bug 1216096, but I don't have debug branch builds to confirm that.

Comment 8

[Mats Palmgren (inactive)]

WFM in my local beta and aurora trees on Linux64.

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Tracked as it's a sec-high

Comment 10

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Jesse, could you please confirm that this issue is fixed as expected on a latest Nightly build? Thanks!

Comment 11

[Al Billings [:abillings - ex-MoCo]]

Did this affect Firefox 42 or ESR38?

Comment 12

[Jonathan Kew [:jfkthame]]

(In reply to Al Billings [:abillings] from comment #11)
> Did this affect Firefox 42 or ESR38?

AFAICS, bug 1164693 landed for FF42, so that will be affected, but not ESR38.

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

Adding tracking flags in case this reopens.

Comment 14

[Jesse Ruderman]

Confirmed WFM on mozilla-central :)

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ba2c446c8f31
Add crashtest.  r=mats

Comment 16

[Cristina Coroiu [:ccoroiu]]

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