Closed Bug 488417 Opened 13 years ago Closed 13 years ago
Crash on table cell select (Ctrl+click) unselect [@ ns
Typed Selection::Remove Range(ns IRange*) ]
Steps:- 1. Open attached "table_select_crash.html" 2. ctrl + click on "A1" to select cell 3. ctrl + click on "B1" to select cell 4. Again ctrl + click on "A1" to unselect cell Result firefox crashes Expected "A1" unselected PASS Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b4pre) Gecko/20090414 Shiretoko/3.5b4pre CRASHES Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090413 Minefield/3.6a1pre
Do you have a crash ID ?
0 xul.dll nsTypedSelection::RemoveRange layout/generic/nsSelection.cpp:5126 1 xul.dll nsFrameSelection::HandleTableSelection layout/generic/nsSelection.cpp:2756 2 xul.dll HandleFrameSelection layout/generic/nsFrame.cpp:2384 3 xul.dll nsFrame::HandleRelease layout/generic/nsFrame.cpp:2461 4 xul.dll nsFrame::HandleEvent layout/generic/nsFrame.cpp:1707 5 xul.dll nsPresShellEventCB::HandleEvent layout/base/nsPresShell.cpp:1384 6 xul.dll xul.dll@0x3c2a19
Summary: Crash on table cell select (Ctrl+click) unselect → Crash on table cell select (Ctrl+click) unselect [@ nsTypedSelection::RemoveRange(nsIRange*) ]
This seems to have regressed somewhere between 2009-04-01 and 2009-04-12. Also, see https://bugzilla.mozilla.org/attachment.cgi?id=372982 which is a test with enhanced privileges, which might be useful for an automated test.
Using Martijn's testcase from bug 488579. Works: http://hg.mozilla.org/mozilla-central/rev/5506e9239176 Crashes: http://hg.mozilla.org/mozilla-central/rev/a10ff1269f64
So it looks like an AddRef has got lost somewhere. nsTypedSelection::RemoveRange calls RemoveItem(aRange). When RemoveItem removes the range from mRanges, the refcount drops to 0, and gets destroyed. After that call, when aRange is dereferenced it is now a dangling pointer...
Graeme, thanks for doing all the hard work here! If someone can think of a way of automating that test, please let me know?
Comment on attachment 373432 [details] [diff] [review] Yeah, indeed. This should do the trick A mochitest could just synthesize mouse events, right?
Whiteboard: [needs landing]
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
Why does this block 1.9.1? It's a regression from 486547, which is trunk only AFAICT
Yeah. This has nothing to do with 1.9.1. It's a regression from a trunk-only cleanup that's staying trunk-only.
Priority: P2 → --
Pushed http://hg.mozilla.org/mozilla-central/rev/da575593579c with a mochitest.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
I had to disable the mochitest, because it made the test _after_ it fail (more or less reliably on mac, intermittently on Windows, never on Linux). roc, it looks like you wrote that test (test_movement_by_characters.html). Any idea why it's failing?
Flags: in-testsuite+ → in-testsuite?
Whiteboard: [needs landing]
And try server doesn't get the orange. I have no idea what's going on with that, and I've filed bug 489560 on reenabling it.
Depends on: 489560
Crash Signature: [@ nsTypedSelection::RemoveRange(nsIRange*) ]
You need to log in before you can comment on or make changes to this bug.