244128 - [FIX] crash tabbing to iframe in print preview [@ nsEventStateManager::ShiftFocusInternal]

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[timeless]

I believe this was while quiting.

  nsCOMPtr<nsISupports> pcContainer = mPresContext->GetContainer();
  NS_ASSERTION(pcContainer, "no container for presContext");
--in a debug build, this would have fired--
  nsCOMPtr<nsIDocShell> docShell(do_QueryInterface(pcContainer));
  nsCOMPtr<nsIDocShellTreeItem> shellItem(do_QueryInterface(docShell));
  shellItem->GetItemType(&itemType);

+	pcContainer.mRawPtr	0x00000000	nsISupports *
+	docShell.mRawPtr	0x00000000	nsISupports *
+	shellItem.mRawPtr	0x00000000	nsISupports *

-	shellItem	{...}	nsCOMPtr<nsIDocShellTreeItem>
+	nsCOMPtr_base	{mRawPtr=0x00000000 }	nsCOMPtr_base

>	gklayout.dll!nsEventStateManager::ShiftFocusInternal(int 
aForward=0x00000001, nsIContent * aStart=0x045e63f0)  Line 3124 + 0x3	C++
 	gklayout.dll!nsEventStateManager::ShiftFocusInternal(int 
aForward=0x00000001, nsIContent * aStart=0x00000000)  Line 3259 + 0xf	C++
 	gklayout.dll!nsEventStateManager::ShiftFocus(int aForward=0x00000001, 
nsIContent * aStart=0x00000000)  Line 3077	C++
 	gklayout.dll!nsEventStateManager::PostHandleEvent(nsIPresContext * 
aPresContext=0x00000000, nsEvent * aEvent=0x012b97fc, nsIFrame * 
aTargetFrame=0x0456a810, nsEventStatus * aStatus=0x00000001, nsIView * 
aView=0x00000000)  Line 2112	C++
 	gklayout.dll!PresShell::HandleEventInternal(nsEvent * 
aEvent=0x0456a810, nsIView * aView=0x0451a7d8, unsigned int aFlags=0x00000001, 
nsEventStatus * aStatus=0x0012f90c)  Line 6125 + 0x19	C++
 	gklayout.dll!PresShell::HandleEvent(nsIView * aView=0x0451a7d8, 
nsGUIEvent * aEvent=0x0012f99c, nsEventStatus * aEventStatus=0x0012f90c, int 
aForceHandle=0x00000001, int & aHandled=0x00000001)  Line 5966 + 0x11	C++
 	gklayout.dll!nsViewManager::HandleEvent(nsView * aView=0x0456a810, 
nsGUIEvent * aEvent=0x00000001, int aCaptured=0x00000000)  Line 2153	C++
 	gklayout.dll!nsViewManager::DispatchEvent(nsGUIEvent * 
aEvent=0x3d888889, nsEventStatus * aStatus=0x0012f960)  Line 1939 + 0x14
	C++
 	gklayout.dll!HandleEvent(nsGUIEvent * aEvent=0x0012f99c)  Line 79
	C++
 	gkwidget.dll!nsWindow::DispatchEvent(nsGUIEvent * event=0x0012f99c, 
nsEventStatus & aStatus=nsEventStatus_eIgnore)  Line 1067 + 0x3	C++
 	gkwidget.dll!nsWindow::DispatchWindowEvent(nsGUIEvent * 
event=0x00000000)  Line 1088	C++
 	gkwidget.dll!nsWindow::DispatchKeyEvent(unsigned int 
aEventType=0x00000083, unsigned short aCharCode=0x0000, unsigned int 
aVirtualCharCode=0x00000009, long aKeyData=0x00000000)  Line 2954 + 0xe	C++
 	gkwidget.dll!nsWindow::OnChar(unsigned int charCode=0x00000009)  Line 
3113 + 0x11	C++
 	gkwidget.dll!nsWindow::ProcessMessage(unsigned int msg=0x00000102, 
unsigned int wParam=0x00000009, long lParam=0x00000000, long * 
aRetValue=0x0012fc88)  Line 3804 + 0xa	C++
 	gkwidget.dll!nsWindow::WindowProc(HWND__ * hWnd=0x001f1862, unsigned 
int msg=0x00000102, unsigned int wParam=0x00000009, long lParam=0x03d64444)  
Line 1349 + 0x10	C++
 	user32.dll!77d43a50() 	
 	user32.dll!77d43b1f() 	
 	user32.dll!TranslateMessage()  + 0xef	
 	user32.dll!GetMessageW()  + 0x125	
 	user32.dll!DispatchMessageW()  + 0xb	
 	appshell.dll!nsAppShellService::Run()  Line 524	C++
 	mozilla.exe!main1(int argc=0x00000001, char * * argv=0x00000000, 
nsISupports * nativeApp=0x00000000)  Line 1302 + 0x9	C++
 	mozilla.exe!main(int argc=0x00000001, char * * argv=0x002a4120)  Line 
1779 + 0x16	C++
 	mozilla.exe!WinMain(HINSTANCE__ * __formal=0x00400000, HINSTANCE__ * 
__formal=0x00400000, char * args=0x0015232a, HINSTANCE__ * 
__formal=0x00400000)  Line 1807 + 0x17	C++
 	mozilla.exe!WinMainCRTStartup()  Line 392 + 0xf	C
 	kernel32.dll!GetCurrentDirectoryW()  + 0x44	

I can't convince devenv to find anything useful

Comment 1

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Stacktrace

Comment 2

[Stephen Donner [:stephend] Not actively reading bugmail]

Steps to reproduce my crash:

1. Load http://www.netscape.com
2. Print Preview
3. Switch from Portrait to Landscape; while switching, press Tab.

Comment 3

[Andrew Schultz]

Attachment: testcase

testcase based on http://www.netscape.com/
linux seamonkey build 2005071005 crashes hitting tab twice after launching
print preview.

Comment 4

[Andrew Schultz]

When it crashes, I get these assertions:
###!!! ASSERTION: nsRenderingContextPS::InvertRect: 'NotYetImplemented', file
/build/andrew/moz-debug/mozilla/gfx/src/ps/nsRenderingContextPS.cpp, line 710
###!!! ASSERTION: nsRenderingContextPS::InvertRect: 'NotYetImplemented', file
/build/andrew/moz-debug/mozilla/gfx/src/ps/nsRenderingContextPS.cpp, line 710
###!!! ASSERTION: no container for presContext: 'pcContainer', file
/build/andrew/moz-debug/mozilla/content/events/src/nsEventStateManager.cpp, line
3131
###!!! ASSERTION: You can't dereference a NULL nsCOMPtr with operator->().:
'mRawPtr != 0', file ../../dist/include/xpcom/nsCOMPtr.h, line 849

Comment 5

[Martijn Wargers (dead)]

Backing out the patch from bug 145388 fixes the crash, but makes that bug appear
again. 
From bug 145388, comment 3:
"This seems to work fine, because the forms controls can never get focus"
Well, that doesn't seem to true anymore for current trunk builds. I can focus
buttons/select/checkboxes/radio buttons by clicking on them.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Aaron, mats, any idea how we could fix this?  The problem is that we can
actually tab around in print preview (eg the first time we hit tab we start
trying to blink the caret in the textfield, hence the InvertRect asserts!). 
When we tab to the iframe, we crash because the relevant event state manager's
prescontext in fact does not have a container (this is nulled out in
nsDocumentViewer when entering print preview).

Now we could just change ShiftFocusInternal to deal with a null container here,
but in general we shouldn't be trying to shift focus within the print-previewing
presentation...

Comment 7

[Mats Palmgren (inactive)]

Attachment: Testcase #2

Comment 8

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

This patch prevents having focus inside the content document, except for
the canvas so that PageUp keys etc still works.
1. reset all ESM focus state when setting up the new presentation
2. catch TAB events and move the focus manually, so we get the following
   tabbing order: document canvas -> items in the Print Preview toolbar
   (so we can directly use PageUp etc on entering PP)
3. prevent ESM for moving focus inside a document in PP mode

Comment 9

[Martijn Wargers (dead)]

Mats, you may also want to somehow include the fix for bug 158362 in this patch,
since you're changing the same code. 

Comment 10

[Brian Ryner (not reading)]

Comment on attachment 189634 [details] [diff] [review]
Patch rev. 1

I think this is ok.  I'd like Boris to have a look too.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 189634 [details] [diff] [review]
Patch rev. 1

>Index: layout/base/nsDocumentViewer.cpp
>+  nsCOMPtr<nsISupports> currentContainer = do_QueryInterface(aDocShell);

No need for that QI; we plan to clobber currentContainer on our call to
GetNext() in the loop anyway.

sr=bzbarsky with that.	Thanks for fixing this!

Comment 12

[Mats Palmgren (inactive)]

Ok, removed the initializer and checked in 2005-07-20 17:15 PDT.

-> FIXED

Comment 13

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Stacktrace

I still crash with build 2005-07-21-06 Windows XP SeaMonkey trunk with
https://bugzilla.mozilla.org/attachment.cgi?id=189627

Comment 14

[Stephen Donner [:stephend] Not actively reading bugmail]

Mats, let me know if you want these two cases spun off:

1) Holding down Tab then clicking Portrait or Landscape crashes
2) Holding down Shift, then pressing Tab while clicking Portrait or Landscape
crashes.

Comment 15

[Mats Palmgren (inactive)]

Attachment: Additional patch, rev. 1

Changing between landscape/portrait is a bit different from creating the first
PP presentation. The first time the galley mode presentation stuff is "cached"
and only has its container reset, the second time we enter here we actually
destroy the presentation (the first PP that is).
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/base/nsDocumentViewer.cpp&rev=1.438&mark=4023-4048&root=/cvsroot#4011


Appearantly the destroy triggers other code to move the focus,
I tried to find the exact place but I got lost in the code...

Anyway, this patch should be pretty safe to add.
I have stress tested on Windows and Linux and everything seems to work now.

Comment 16

[Mats Palmgren (inactive)]

Stephen, thanks for catching the additional cases!

Checked in "Additional patch, rev. 1" to trunk 2005-07-25 17:55 PDT

-> FIXED

Comment 17

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED with build 2005-07-26-05 on Windows XP SeaMonkey trunk.