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

RESOLVED FIXED in mozilla22

Status

()

RESOLVED FIXED
8 years ago
3 months ago

People

(Reporter: mats, Unassigned)

Tracking

({assertion})

Trunk
mozilla22
x86
Linux
assertion
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fuzzblocker])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

8 years ago
(Rev3 Fedora 12 tryserver debug test mochitest-other on 2011/02/25 20:08:09)

chrome://mochitests/content/a11y/accessible/editabletext/test_1.html
triggered this assertion twice:

###!!! ASSERTION: Invalid offset: 'aOffset <= mSkipChars->mCharCount', file ../../../gfx/thebes/gfxSkipChars.cpp, line 92
gfxSkipCharsIterator::SetOffsets [gfxSkipChars.cpp:94]
gfxSkipCharsIterator::SetSkippedOffset [gfxSkipChars.h:275]
gfxSkipCharsIterator::ConvertSkippedToOriginal [gfxSkipChars.h:283]
nsHyperTextAccessible::RenderedToContentOffset [nsHyperTextAccessible.cpp:2165]
nsHyperTextAccessible::GetDOMPointByFrameOffset [nsHyperTextAccessible.cpp:2304]
nsHyperTextAccessible::HypertextOffsetsToDOMRange [nsHyperTextAccessible.cpp:770]
nsHyperTextAccessible::SetSelectionBounds [nsHyperTextAccessible.cpp:1945]
nsHyperTextAccessible::SetSelectionRange [nsHyperTextAccessible.cpp:1598]
nsHyperTextAccessible::DeleteText [nsHyperTextAccessible.cpp:1525]
nsHyperTextAccessible::SetTextContents [nsHyperTextAccessible.cpp:1471]
xptiInterfaceEntry::EnsureResolved [xptiprivate.h:264]
CallMethodHelper::Invoke [xpcwrappednative.cpp:3124]
CallMethodHelper::Call [xpcwrappednative.cpp:2390]
XPCWrappedNative::CallMethod [xpcwrappednative.cpp:2354]
XPC_WN_CallMethod [xpcwrappednativejsops.cpp:1613]
js::CallJSNative [jscntxtinlines.h:701]
js::Interpret [jsinterp.cpp:4798]
js::RunScript [jsinterp.cpp:653]
js::Invoke [jsinterp.cpp:740]
js::ExternalInvoke [jsinterp.cpp:863]
JS_CallFunctionValue [jsapi.cpp:5173]
nsXPCWrappedJSClass::CallMethod [xpcwrappedjsclass.cpp:1672]
nsXPCWrappedJS::CallMethod [xpcwrappedjs.cpp:588]
PrepareAndDispatch [xptcstubs_gcc_x86_unix.cpp:95]
nsThread::ProcessNextEvent [nsThread.cpp:633]
NS_ProcessNextEvent_P [nsThreadUtils.cpp:250]
mozilla::ipc::MessagePump::Run [MessagePump.cpp:110]
MessageLoop::RunInternal [message_loop.cc:220]
MessageLoop::RunHandler [message_loop.cc:203]
MessageLoop::Run [message_loop.cc:176]
nsBaseAppShell::Run [nsBaseAppShell.cpp:198]
nsAppStartup::Run [nsAppStartup.cpp:220]
XRE_main [nsAppRunner.cpp:3781]
main [nsBrowserApp.cpp:158]


Also occurs on mozilla-central:
Rev3 Fedora 12 mozilla-central debug test mochitest-other on 2011/03/01 13:11:15
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1299013875.1299017568.10848.gz&fulltext=1

Updated

7 years ago
Blocks: 571613

Comment 2

6 years ago
This one is kind of bad.  I don't want to ignore this assertion, and I don't want to disable testing of accessibility.
Whiteboard: [fuzzblocker]
Created attachment 722623 [details] [diff] [review]
patch

this seems more correct, but I can't find a case this fixes other than test_1.html doesn't make us assert anymore.  I suspect bug 452584  may or not be the same issue, but the tests in test_1.html that are theoretically disabled because of that bug don't pass with this patch so...
Attachment #722623 - Flags: review?(surkov.alexander)
Created attachment 722625 [details] [diff] [review]
patch

first one was only test
Attachment #722623 - Attachment is obsolete: true
Attachment #722623 - Flags: review?(surkov.alexander)
Attachment #722625 - Flags: review?(surkov.alexander)
Trev, can you describe in details why does it happen please?
(In reply to alexander :surkov from comment #5)
> Trev, can you describe in details why does it happen please?

so, the first time through the loop we start startAcc and startFrame, and leave the end things pointing to null.  The next time through we have a HTMLBRAccessible which isn't a text accessible so we don't go through the path that would set endAcc / endFrame because nsAccUtils::IsText() returns false.  Then we break out of the loop because that was all of the range, but endAcc / endFrame are still null so we set them to be the same as start.  (I'm pretty sure that's the way it works, but would need to check tomorrow to for more detail).
Comment on attachment 722625 [details] [diff] [review]
patch

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

I think this code should return wrong aEndOffset in described case but I would rather rewrite this code from scratch than trying to fix it.

::: accessible/src/generic/HyperTextAccessible.cpp
@@ +233,5 @@
>    else if (startOffset > endOffset) {
>      return nullptr;
>    }
>  
> +  nsIFrame *startFrame = nullptr, * endFrame = nullptr; if (aEndFrame) {

nit: nsIFrame* name

if (aEndFrame) must be on own line
Attachment #722625 - Flags: review?(surkov.alexander) → review+
(In reply to alexander :surkov from comment #7)
> Comment on attachment 722625 [details] [diff] [review]
> patch
> 
> Review of attachment 722625 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I think this code should return wrong aEndOffset in described case but I
> would rather rewrite this code from scratch than trying to fix it.
> 
> ::: accessible/src/generic/HyperTextAccessible.cpp
> @@ +233,5 @@
> >    else if (startOffset > endOffset) {
> >      return nullptr;
> >    }
> >  
> > +  nsIFrame *startFrame = nullptr, * endFrame = nullptr; if (aEndFrame) {
so do we want to check this in?

> nit: nsIFrame* name
> 
> if (aEndFrame) must be on own line

yeah, not sure how that happened..
(In reply to Trevor Saunders (:tbsaunde) from comment #8)

> > I think this code should return wrong aEndOffset in described case but I
> > would rather rewrite this code from scratch than trying to fix it.
> > 

> so do we want to check this in?

I think so, no known regressions, it fixes assertion. I plan to touch this code soon anyway.

> > > +  nsIFrame *startFrame = nullptr, * endFrame = nullptr; if (aEndFrame) {

btw, pls, keep startFrame and endFrame definitions on separate lines
Follow-up fix to editabletext/test_1.html annotations.
https://hg.mozilla.org/integration/mozilla-inbound/rev/79b8e0a0bdb7
https://hg.mozilla.org/mozilla-central/rev/397034d19dfc
https://hg.mozilla.org/mozilla-central/rev/79b8e0a0bdb7
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
(Reporter)

Updated

3 months ago
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.