"ASSERTION: Invalid offset" with bidi text change

RESOLVED WORKSFORME

Status

()

defect
RESOLVED WORKSFORME
4 years ago
Last year

People

(Reporter: jruderman, Unassigned)

Tracking

(Blocks 1 bug, 4 keywords)

Trunk
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox42 wontfix, firefox43+ fixed, firefox44+ fixed, firefox45+ fixed, firefox-esr38 unaffected)

Details

(Whiteboard: [fixed on trunk (45) by bug 1216096][adv-main43+])

Attachments

(2 attachments)

Reporter

Description

4 years ago
Posted file 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
Reporter

Comment 1

4 years ago
Posted file stack
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.
Keywords: sec-high

Comment 3

4 years ago
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)
Has Regression Range: --- → yes
Has STR: --- → yes
Depends on: 1216096
Flags: needinfo?(smontagu)
Flags: in-testsuite?
Whiteboard: [fixed on trunk (45) by bug 1216096]
(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.
Flags: needinfo?(smontagu)

Comment 5

4 years ago
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.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WORKSFORME

Comment 6

4 years ago
Bug 1164693 is probably the right bug number for the bug that originally regressed it.
Blocks: 1164693
Keywords: regression
Should be fixed43 and fixed44 by the uplift of bug 1216096, but I don't have debug branch builds to confirm that.

Comment 8

4 years ago
WFM in my local beta and aurora trees on Linux64.
Tracked as it's a sec-high
Jesse, could you please confirm that this issue is fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(jruderman)
Did this affect Firefox 42 or ESR38?
(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.
Adding tracking flags in case this reopens.
Reporter

Comment 14

4 years ago
Confirmed WFM on mozilla-central :)
Flags: needinfo?(jruderman)
Whiteboard: [fixed on trunk (45) by bug 1216096] → [fixed on trunk (45) by bug 1216096][adv-main43+]
Group: layout-core-security

Updated

Last year
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.