Closed
Bug 179089
Opened 22 years ago
Closed 21 years ago
crash in selection stuff loading a newsgroup message [@JS_GetPrivate]
Categories
(Core :: DOM: Selection, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: timeless, Assigned: mjudge)
Details
(Keywords: crash)
Crash Data
Attachments
(1 file)
8.00 KB,
patch
|
caillon
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
I was reading news.openwatcom.org/openwatcom.contributors
i use n/space to advance messages and i also double click in the message body
sometimes for fear that focus is in the wrong place. the response time for this
session was bad, i'm not sure if that contributed or why it would be the case.
Comment 1•22 years ago
|
||
JS_GetPrivate [d:/builds/seamonkey/mozilla/js/src/jsapi.c, line 1928]
nsScriptSecurityManager::GetFunctionObjectPrincipal
[d:/builds/seamonkey/mozilla/caps/src/nsScriptSecurityManager.cpp, line 1808]
nsScriptSecurityManager::GetFramePrincipal
[d:/builds/seamonkey/mozilla/caps/src/nsScriptSecurityManager.cpp, line 1842]
nsScriptSecurityManager::GetPrincipalAndFrame
[d:/builds/seamonkey/mozilla/caps/src/nsScriptSecurityManager.cpp, line 1855]
nsScriptSecurityManager::GetSubjectPrincipal
[d:/builds/seamonkey/mozilla/caps/src/nsScriptSecurityManager.cpp, line 1895]
nsScriptSecurityManager::GetSubjectPrincipal
[d:/builds/seamonkey/mozilla/caps/src/nsScriptSecurityManager.cpp, line 1607]
nsContentUtils::CanCallerAccess
[d:/builds/seamonkey/mozilla/content/base/src/nsContentUtils.cpp, line 531]
nsRange::SetStart [d:/builds/seamonkey/mozilla/content/base/src/nsRange.cpp,
line 1030]
nsTypedSelection::SetOriginalAnchorPoint
[d:/builds/seamonkey/mozilla/content/base/src/nsSelection.cpp, line 6614]
nsTypedSelection::Collapse
[d:/builds/seamonkey/mozilla/content/base/src/nsSelection.cpp, line 6151]
nsSelection::TakeFocus
[d:/builds/seamonkey/mozilla/content/base/src/nsSelection.cpp, line 2853]
nsSelection::HandleClick
[d:/builds/seamonkey/mozilla/content/base/src/nsSelection.cpp, line 2643]
nsFrame::HandlePress
[d:/builds/seamonkey/mozilla/layout/html/base/src/nsFrame.cpp, line 1499]
nsFrame::HandleEvent
[d:/builds/seamonkey/mozilla/layout/html/base/src/nsFrame.cpp, line 981]
PresShell::HandleEventInternal
[d:/builds/seamonkey/mozilla/layout/html/base/src/nsPresShell.cpp, line 6263]
PresShell::HandleEvent
[d:/builds/seamonkey/mozilla/layout/html/base/src/nsPresShell.cpp, line 6169]
nsViewManager::HandleEvent
[d:/builds/seamonkey/mozilla/view/src/nsViewManager.cpp, line 2209]
nsView::HandleEvent [d:/builds/seamonkey/mozilla/view/src/nsView.cpp, line 304]
nsViewManager::DispatchEvent
[d:/builds/seamonkey/mozilla/view/src/nsViewManager.cpp, line 1949]
HandleEvent [d:/builds/seamonkey/mozilla/view/src/nsView.cpp, line 83]
nsWindow::DispatchEvent
[d:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 1074]
nsWindow::DispatchWindowEvent
[d:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 1091]
nsWindow::DispatchMouseEvent
[d:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 5208]
nsWindow::DispatchFocus
[d:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 5462]
nsWindow::ProcessMessage
[d:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 4060]
nsWindow::WindowProc
[d:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 1339]
USER32.DLL + 0x3eb0 (0x77e13eb0)
USER32.DLL + 0x401a (0x77e1401a)
USER32.DLL + 0x92da (0x77e192da)
nsAppShellService::Run
[d:/builds/seamonkey/mozilla/xpfe/appshell/src/nsAppShellService.cpp, line 472]
main1 [d:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1538]
main [d:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1886]
WinMain [d:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1906]
WinMainCRTStartup()
KERNEL32.DLL + 0x7903 (0x77e87903)
1927 brendan 3.114 v = GC_AWARE_GET_SLOT(cx, obj, JSSLOT_PRIVATE);
(cc for help and verification of analysis, not blame)
I'm guessing the crash is a destroyed obj.
nsScriptSecurityManager::GetFramePrincipal
appears to verify that obj isn't null
wow, no mailnews code at all. well here's what i think happened:
mozilla was slowed down (and handling events slowly).
so a mouse double click event fired by the old mailnews message was processed
after the message was replaced.
Summary: crash in selection stuff loading a newsgroup message [@JS_GetPrivate] tb13679222k → crash in selection stuff loading a newsgroup message [@JS_GetPrivate]
Comment 3•21 years ago
|
||
I think we should move this bug out from under browser / selection. I don't
think it has to to with selection.
You can reproduce this in the latest thunderbird builds very easily.
1) bring up mail compose
2) Attach a web page, type in a url
3) hit the send button
4) we crash with this same stack trace.
Although I just noticed this bug was reported back in 2002. Maybe thunderbird
just introduced a new scenario that leads back to this same crash.
Comment 4•21 years ago
|
||
bz apparently fixed this problem for the insert case, but the remove case
wasn't addressed.
The code is nearly identical for the two functions so I moved the common code
to a separate local function to help prevent future maintenance headaches.
Comment 5•21 years ago
|
||
Oh, I guess some explanation would be good too.
The NS_RELEASE was causing problems because the pointer can be null, as
indicated by surrounding code. This referencing of a null pointer was getting
caught by Win32's structured exception handling. Fixing this, I couldn't
reproduce There's two possibilities from here.
1. The caught exception left objects in an odd state and this was the source of
the faults in JS
2. Correcting this problem effectively hides what's causing these crashes
There's still a lot of asserts firing, but I was getting those before.
Comment 6•21 years ago
|
||
That's great news David. Thanks a lot for the quick turn around on this. I'll
try your patch out in thunderbird today and verify that it also works for me.
Comment 7•21 years ago
|
||
Comment on attachment 127462 [details] [diff] [review]
Fixes the null pointer and consolidates the code
- In NotifyListBoxBody(nsIPresContext* aPresContext,
+ nsIContent* aContainer,
+ nsIContent* aChild,
+ PRInt32 aIndexInContainer,
+ nsIDocument* aDocument,
+ nsIFrame* aChildFrame,
+ PRBool useXBLForms,
+ content_operation operation)
Maybe rename useXBLForms to aUseXBLForms and operation to aOperation to be
consistent with the other arguments?
+{
+ if (aContainer) {
...
}
+ return PR_FALSE;
How about:
+{
+ if (!aContainer) {
+ return PR_FALSE;
+ }
+ ...
to avoid indenting the whole function?
+ PRBool listitem = tag && tag.get() == nsXULAtoms::listitem;
+ if (listitem) {
|listitem| isn't used for anything except that if check, maybe loose it, and
change the above to:
+ if (tag == nsXULAtoms::listitem;
(no need for the .get(), and nsXULAtoms::listitem isn't null unless something
went really wrong during startup, in which case we should never get here).
- In nsCSSFrameConstructor::ContentInserted():
+ if (aContainer && NotifyListBoxBody(aPresContext, aContainer, aChild,
+ aIndexInContainer, mDocument, nsnull,
+ UseXBLForms(), CONTENT_INSERTED))
NotifyListBoxBody returns false if (!aContainer), so no need to check it here
before making the call.
- In nsCSSFrameConstructor::ContentRemoved():
+ if (aContainer && NotifyListBoxBody(aPresContext, aContainer, aChild,
+ aIndexInContainer, mDocument,
childFrame,
+ UseXBLForms(), CONTENT_REMOVED))
Same thing, no need to check aContainer.
sr=jst
Attachment #127462 -
Flags: superreview+
Comment 8•21 years ago
|
||
Who will r=? Should we get this into 1.5a for some widely-distributed testing?
/be
Comment 9•21 years ago
|
||
Comment on attachment 127462 [details] [diff] [review]
Fixes the null pointer and consolidates the code
What jst said, and...
>+static
>+PRBool NotifyListBoxBody(nsIPresContext* aPresContext,
....
>+ } else {
>+ // There is no frame for the removed/inserted content, so we need to dig
>+ // further for the body frame. XBL insertion may not yet have taken
>+ // place here (in the case of replacements), so our only connection
Nit: Could you remove the weird double space in between "of" and
"replacements"?
>+ // to the listbox content is through aContainer. Use the boxObject
>+ // to get to the listboxbody frame so we can notify it of the removal.
This comment applies to both insertion and removal cases. Please reflect that
in the last line of the comment (it currently pertains to only removal).
Attachment #127462 -
Flags: review+
Comment 10•21 years ago
|
||
Should I seek approval for 1.5a? Given that this is blocking a feature for
Thunderbird and might be responsible for some other similiar screwy crashes it
seems like a decent win. I also noticed about a crash a day listed for the
ContentRemoved method as well.
Status: NEW → ASSIGNED
Comment 11•21 years ago
|
||
Patch checked in
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
Crash Signature: [@JS_GetPrivate]
You need to log in
before you can comment on or make changes to this bug.
Description
•