Build id: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050703 Firefox/1.0+ Now that bug 126719 is worksforme I noticed that F5 still works, but in a very special case Steps to reproduce: 1. Visit testcase 2. Go to Print Preview 3. Press F5 or Ctrl + r, nothing will h appen 4. Right click several time on the page 5. Press F5 and notice it will reload and mess up PP

Just focusing something in the chrome area (input or select or button) and then pressing F5 allways shows the bug to me. Alo Esc, F1, and F11 work in print preview.

This fixes the F5 reload bug, but I would have expected that reloading a document would follow this code path: http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.cpp#3079 The Esc + Ctrl-W bug might have something to do with the code here: http://lxr.mozilla.org/seamonkey/source/toolkit/components/printing/content/printUtils.js#261 but I'm not really sure about that.

No, wait, I get an error message in the js console with the escape key: * Call to xpconnect wrapped JSObject produced this error: * [Exception... "'[JavaScript Error: "navigate is not defined" {file: "chrome://br owser/content/browser.xul" line: 1}]' when calling method: [nsIDOMEventListener: :handleEvent]" nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAIL S)" location: "<unknown>" data: yes]

Ok, it happens because of the onblur="navigate" code. I can't just remove it, because it would regress bug 126726. But I can solve it this way. I think the F1 key should just be allowed to work. Maybe the F11 key should be disabled, but it doesn't really do that much harm, afaik.

Comment on attachment 188711 [details] [diff] [review] patch for escape Try changing the onblur to onchange instead?

Yes, that's a much better idea and it works. Thanks. Uhm, but I can't really say I made this patch.

Comment on attachment 188698 [details] [diff] [review] patch Maybe you can also review this?

Comment on attachment 188860 [details] [diff] [review] patch2 for escape >- onblur="navigate(0, this.value, 0);" >+ onchange="navigate(0, this.value, 0);" > onkeypress="if (event.keyCode==13) navigate(0, this.value, 0);"/> Well, I had a look at this, and it would seem that the old code (for bug 126726) hasn't worked since the checkin for bug 147058. But looking at the code more closely (sorry for overlooking it last time) I'd like to point out that onchange does in fact fire for DOM_VK_RETURN keypresses as well, thus you should find that the fix also makes the onkeypress attribute redundant.

You're right, again. (I thought I checked if I could remove this, oh well..)

Comment on attachment 188698 [details] [diff] [review] patch Er, I accidently made this obsolete. Neil, this patch is for fixing the F5 reload issue. I hope you can review this too.

Neil, you might not have noticed the other patch, that's why I'm cc-ing you. Could you review that also? Thanks!

Comment on attachment 188698 [details] [diff] [review] patch Sorry, I don't know enough about the doc shell to review this.

Comment on attachment 188698 [details] [diff] [review] patch Don't we need to also check whether some ancestor is printing/pp?

And in any case, if we're printing don't we want to handle the load once printing is done?

(In reply to comment #14) > Don't we need to also check whether some ancestor is printing/pp? I I understand it correctly, all the docShells are set to mIsPrinting: http://lxr.mozilla.org/seamonkey/source/layout/base/nsDocumentViewer.cpp#3773 (In reply to comment #15) > And in any case, if we're printing don't we want to handle the load once > printing is done? Well, I just copied this code from: http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.cpp#3080 But you mean you want the load to happen after printing/print preview has done? Wouldn't that be very odd?

> I I understand it correctly, all the docShells are set to mIsPrinting: Ah, ok. Check to make sure via printf or something, but looks like it... > But you mean you want the load to happen after printing/print preview has done? > Wouldn't that be very odd? For print preview, yes. For printing, no (imo).

Well, I tried to use something like this: nsCOMPtr<nsIContentViewer> kungfuDeathGrip = mContentViewer; if (mContentViewer) { nsCOMPtr<nsIDocumentViewerPrint> docviewer = do_QueryInterface(mContentViewer); if (docviewer) { if (docviewer->GetIsPrintPreview()) { return NS_OK; // JS may not handle returning of an error code } } } to get only print preview, but that doesn't work. Complaints about nsIDocumentViewerPrint not existing. Any ideas to check if only printpreview is turned on? The other patch - "patch3 for escape" - can be checked in, by the way, if anyone likes that (it's more Neil's patch than mine).

> Complaints about nsIDocumentViewerPrint not existing. Did you include nsIDocumentViewerPrint.h? In any case, I think I'd like some UI-folks buy-in on what should happen here for print....

(In reply to comment #19) > Did you include nsIDocumentViewerPrint.h? Yes, when I do that, it also complains that that file doesn't exist.

Ah, that's not exported out of layout/base.... That would do it.

For clarity's sake, "patch3 for escape" was checked in at 2005-07-19 13:43.

So is this bug fixed, then?

(In reply to comment #23) > So is this bug fixed, then? No, the original bug is still there.

*** Bug 325349 has been marked as a duplicate of this bug. ***

observed on linux as well. resolvable with: https://bugzilla.mozilla.org/show_bug.cgi?id=133787 (as noted by aleksej)

Discovered this bug today during QA. I reproduced it by having print preview open and launching a link from another application (Mirc in this case) and when it opened a new tab in Firefox it opened it inside the print preview pane and closing it proved difficult. Confirmed in FF 1.5.0.8 RC2 and the current Bon Echo build on Windows XP x64. The F5 bug still seems present although I find it only really happens with two or more tabs open. Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.0.8) Gecko/20061025 Firefox/1.5.0.8 - Build ID: 2006102516

*** Bug 363778 has been marked as a duplicate of this bug. ***

I crash now in current trunk build with the steps to reproduce. I filed bug 396024 for that, since a fix for this, doesn't necessarily has to fix that crash. But either way, it's probably important to prevent this crash from occuring this way, so I think this bug and/or bug 396024 needs to be fixed for Firefox3.

Ok, I finally know how to export from layout/base. I'm not sure if this is the right solution, though. Why shouldn't someone from script allow to reload a document in print preview?

Comment on attachment 280747 [details] [diff] [review] docshellpatch This looks about right, but no need for that export and the complicated code. Just change the mFiredUnloadEvent check to be more like the check in GoBack() and company.

i'm not really sure what the impact of this is and whether it should be blocking or not. bz thoughts?

> i'm not really sure what the impact of this is Most likely crashes under the control of websites if a site does window.open and then you print preview in the window it opened. I think we should block on the patch from comment 33 as adjusted to comment 34.

You mean like this? But I'm confused about this, because this is basically what my first patch is, but you didn't want that because you wanted printing to not behave like this.

Comment on attachment 282302 [details] [diff] [review] patch? >Index: docshell/base/nsDocShell.cpp >+ if (mFiredUnloadEvent || !IsNavigationAllowed()) { That should just be |if (!IsNavigationAllowed()) {| And yes, I think we could do better. But for 1.9 it's apparently not happening....

Ok, thanks. (this is not really my patch, but anyway)

Comment on attachment 282319 [details] [diff] [review] patch r=bzbarsky

Checking in nsDocShell.cpp; /cvsroot/mozilla/docshell/base/nsDocShell.cpp,v <-- nsDocShell.cpp new revision: 1.859; previous revision: 1.858 done Checked into trunk.

Verified fixed, using: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007092904 Minefield/3.0a9pre

After closing Print dialog ,navigation keys still works on: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008010813 Minefield/3.0b3pre

(In reply to comment #42) > After closing Print dialog ,navigation keys still works on: > Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008010813 > Minefield/3.0b3pre I'm not sure what you mean. Is there still a problem here?