Closed Bug 759858 Opened 8 years ago Closed 8 years ago

crash in nsTypedSelection::AddItem

Categories

(Core :: DOM: Editor, defect, critical)

All
macOS
defect
Not set
critical

Tracking

()

RESOLVED DUPLICATE of bug 771873

People

(Reporter: sheppy, Unassigned)

References

Details

(Keywords: crash, Whiteboard: DUPEME?)

Crash Data

This bug was filed from the Socorro interface and is 
report bp-e7ee3164-03fc-4524-b7bb-64a162120530 .
============================================================= 
This happens to me quite frequently when applying a style to a selection in CKEditor on MDN. It's pretty annoying.
Hmm, I think I've seen stacks like this, but it's very hard to fix this without having STRs.  Do you have any idea what you're doing which triggers this crash?
It *tends* to happen when I'm doing more than one substring at a time, selecting text and adding <code>...</code> around it. It also seems more often to happen when the selected text includes non-alphanumeric characters (such as the string "JSObject *" or "SomeFunction()"), but I don't think that's always the case.

The text in the editor does not have to be long for it to happen, but it tends to be the case that it will happen repeatably on the same text (although not forever; eventually it will work, if I keep restarting and trying again).
When you say more than one substring, do you mean selecting multiple ranges?
No, sorry, I mean one after another. Select a bit of text (usually just a few characters, a dozen or two dozen at most), apply the <code> wrapper, move on to the next one. Doesn't seem to matter how I select (whether by clicking and dragging, clicking and extending the selection using the keyboard, etc).
I played around a bit doing this kind of stuff before, but I'm afraid I've never been able to reproduce this.
Keywords: testcase-wanted
Whiteboard: DUPEME?
Sigh. It happens to me often enough to drive me crazy but I can't get you a testcase either, because of how unreliably it happens. Frustrating!
I'm looking at the code a bit here, and it looks like somehow I'm getting through the checks that should be bouncing me out if I'm not adding a table cell selection, which I'm not. I'm just changing the contents of a range of text.

I'm guessing from the stack that this is happening during the re-check of the selection for spelling after applying the style (which is probably not actually necessary, but I doubt the editor knows that). I will attach a debugger next time it happens and get a better look at what's happening. I'm a little embarrassed that hadn't occurred to me before.
Another possibly useful tidbit: I *think* this only happens on words that are flagged as misspelled by the as-you-type spell checker.
Do you happen to do Ctrl+clicks at all when this happens?

Note that this is most likely happening because the selection here <http://mxr.mozilla.org/mozilla-central/source/extensions/spellcheck/src/mozInlineSpellChecker.cpp#1489> is null.  If you could do a local build, it would be interesting to change this line <http://mxr.mozilla.org/mozilla-central/source/extensions/spellcheck/src/mozInlineSpellChecker.cpp#1460> to:

NS_ENSURE_STATE(spellCheckSelection);

And see if the crashes go away that way (although I don't see why spellCheckSelection should be null there...)
Hm, no control-clicks but I am using control-o (a keyboard shortcut configured in CKEditor on MDN) to wrap the selection in a <code> block.

I will set about doing a local build to test on.
Cool, let me know how I can help.  :-)
Dupe of bug 722039?
(In reply to Scoobidiver from comment #12)
> Dupe of bug 722039?

That is similar, but I don't think this is a dupe.
Just spent some time with ehsan debugging; a transcript of our discussion and the gdb output is here:

https://etherpad.mozilla.org/boBSAACJaB

Basically, it looks like a bug in GetIndicesForInterval, where somehow the end index is getting returned lower than the start index. He says he'll have time to look at it next week.
Assignee: nobody → ehsan
Depends on: 722039
I have not yet gotten a chance to work on this.  Aryeh, any chance you could please do that when you get a chance?  Thanks!
Assignee: ehsan → nobody
(In reply to Eric Shepherd [:sheppy] from comment #14)
> Basically, it looks like a bug in GetIndicesForInterval, where somehow the
> end index is getting returned lower than the start index. He says he'll have
> time to look at it next week.

This sounds exactly like bug 771873.  Does it still happen, or did the patch there fix it (landed July 13)?
(In reply to :Aryeh Gregor from comment #16)
> (In reply to Eric Shepherd [:sheppy] from comment #14)
> > Basically, it looks like a bug in GetIndicesForInterval, where somehow the
> > end index is getting returned lower than the start index. He says he'll have
> > time to look at it next week.
> 
> This sounds exactly like bug 771873.  Does it still happen, or did the patch
> there fix it (landed July 13)?

Can you cc me on that so I can look at it please? I get access denied on that bug.
FWIW, I cannot immediately reproduce this but that doesn't necessarily mean anything. :)
(In reply to Eric Shepherd [:sheppy] from comment #17)
> Can you cc me on that so I can look at it please? I get access denied on
> that bug.

Done.  Sorry for not thinking of that myself!
I suspect this is indeed fixed by that bug, from looking at it. I'm inclined to mark this as resolved:fixed or dupe of that, and re-open if this recurs. Thoughts?
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: CVE-2012-3961
You need to log in before you can comment on or make changes to this bug.