Closed
Bug 704758
Opened 13 years ago
Closed 12 years ago
"ASSERTION: null frame" in PresShell::HandleEvent with focus, navigation
Categories
(Core :: DOM: Events, defect)
Core
DOM: Events
Tracking
()
RESOLVED
FIXED
mozilla12
Tracking | Status | |
---|---|---|
firefox11 | - | --- |
People
(Reporter: jruderman, Assigned: enndeakin)
Details
(Keywords: assertion, testcase)
Attachments
(3 files, 2 obsolete files)
1. Set dom.disable_window_flip to false 2. Load the testcase 3. Click the button. ###!!! ASSERTION: null frame: 'aFrame', file layout/base/nsPresShell.cpp, line 5727
Reporter | ||
Comment 1•13 years ago
|
||
Comment 2•13 years ago
|
||
Is this a regression or did we used to get "null view" assertion?
Assignee | ||
Comment 3•13 years ago
|
||
The issue here is that this is an IME event that expects to be retargeted to the focused window (the code near the top of nsPresShell::HandleEvent). The recursive call to HandleEvent is called with a null frame and the old code which used to handle getting the frame from a parent view was moved to the view manager.
Assignee | ||
Comment 4•13 years ago
|
||
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Assignee | ||
Comment 5•13 years ago
|
||
Comment on attachment 576531 [details] [diff] [review] patch NS_IsEventTargetedAtFocusedWindow only differs from the check done at http://mxr.mozilla.org/mozilla-central/source/view/src/nsViewManager.cpp#996 (this code used to be in the presshell) because there is an extra check for NS_IS_CONTENT_COMMAND_EVENT. Not sure if the extra check for these events are needed. masayuki, this assertion happens during the NS_QUERY_TEXT_CONTENT event. Can you check this over to make this event is still working properly with the patch, or let me know how to test it?
Attachment #576531 -
Flags: review?(bugs)
Attachment #576531 -
Flags: feedback?(masayuki)
Comment 6•13 years ago
|
||
If the event is NS_QUERY_TEXT_CONTENT and the root frame hasn't been in the presShell, you can just return. Then, the query event indicates failed. It's better for widget rather than succeeded on unexpected view. I'll fix it on cocoa widget side. (The caller (IMEInputHandler::ResetIMEWindowLevel()) should retry it later) If the event is a content command event, maybe, we can call nsEventStateManager::DoContentCommandEvent() or nsEventStateManager::DoContentCommandScrollEvent() directly. I think that I should check it later. Your patch should be safe for content command event even though it failed to return correct value. However, I think that you should just return like query event. So, I think that you should just return if the event is content command event or query content event.
Comment 7•13 years ago
|
||
Comment on attachment 576531 [details] [diff] [review] patch if (expr) { stmt; } But based on comment from Masayuki, I'm expecting a new patch here.
Attachment #576531 -
Flags: review?(bugs)
Comment 8•13 years ago
|
||
Neil, will you update the patch?
Assignee | ||
Comment 9•13 years ago
|
||
Attachment #576531 -
Attachment is obsolete: true
Attachment #576531 -
Flags: feedback?(masayuki)
Attachment #579302 -
Flags: review?(bugs)
Comment 10•13 years ago
|
||
Comment on attachment 579302 [details] [diff] [review] updated patch ># HG changeset patch ># Parent 8367f06e227bbdb752d56e8f11ca4a4554c10417 > >diff --git a/layout/base/nsPresShell.cpp b/layout/base/nsPresShell.cpp >--- a/layout/base/nsPresShell.cpp >+++ b/layout/base/nsPresShell.cpp >@@ -5784,17 +5784,36 @@ PresShell::HandleEvent(nsIFrame * > } > > if (retargetEventDoc) { > nsCOMPtr<nsIPresShell> presShell = retargetEventDoc->GetShell(); > if (!presShell) > return NS_OK; > > if (presShell != this) { >- return presShell->HandleEvent(presShell->GetRootFrame(), aEvent, true, aEventStatus); >+ nsIFrame* frame = presShell->GetRootFrame(); >+ if (!frame) { >+ if (aEvent->message == NS_QUERY_TEXT_CONTENT || >+ NS_IS_CONTENT_COMMAND_EVENT(aEvent)) >+ return NS_OK; if (expr) { stmt; } Though, looks like the code around isn't consistent. >+ >+ nsIView* view = presShell->GetViewManager()->GetRootView(); >+ while (view && !view->GetFrame()) { >+ view = view->GetParent(); >+ } >+ >+ if (view) { >+ frame = view->GetFrame(); >+ } >+ } >+ >+ if (!frame) >+ return NS_OK; >+ >+ return presShell->HandleEvent(frame, aEvent, true, aEventStatus); So, frame may not be in presShell? That would be bad. Should you retarget to frame->PresShell()?
Attachment #579302 -
Flags: review?(bugs) → review-
Updated•12 years ago
|
tracking-firefox11:
--- → ?
Assignee | ||
Comment 11•12 years ago
|
||
Attachment #579302 -
Attachment is obsolete: true
Attachment #582868 -
Flags: review?(bugs)
Updated•12 years ago
|
Attachment #582868 -
Flags: review?(bugs) → review+
Comment 12•12 years ago
|
||
(FWIW: I hit this assertion in my linux debug build if I visit file:/// and hold down Ctrl+R to reload over & over. I tried out the latest patch here, and it appears to fix it - woot!)
Updated•12 years ago
|
OS: Mac OS X → All
Hardware: x86_64 → All
Comment 13•12 years ago
|
||
another way to trigger the assert is to enter about: in the url bar and hit enter.
Assignee | ||
Comment 14•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f4e51ad29002
Flags: in-testsuite-
Comment 15•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f4e51ad29002
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
Comment 16•12 years ago
|
||
It's unclear that this is a common user scenario. Please re-nominate if that's not the case.
Comment 17•12 years ago
|
||
Neil, do you know if this bug has any effects in opt builds?
You need to log in
before you can comment on or make changes to this bug.
Description
•