703 bytes, text/html
408 bytes, text/html
3.51 KB, text/plain
12.41 KB, patch
|Details | Diff | Splinter Review|
I neglected to mention that Firefox appears to navigate to the new page indicated as instructed by the onclick handler in the page. The title of the of the window changes to that of the mozilla.org site, before the crash occurs (possibly when it attempts to render the page and has nothing to render into?).
Created attachment 149572 [details] On-line testcase
in a Linux debug build, this assertion shows up when clicking on the link in print preview: ###!!! ASSERTION: Someone did not call nsIPresShell::destroy: 'Not Reached', file nsPresShell.cpp, line 1691 Break: at file nsPresShell.cpp, line 1691 The stack is empty though. No dupes found, marking NEW.
The problem is probably that print preview sets a listener to cancel events on the toplevel window only... and events don't bubble/capture across frames within the content hierarchy. Print preview needs to be setting the listener on the <browser> containing the content or on each frame in the window.
bz, any idea as to which listener? The one I guessed at wasn't it :-/
Not offhand... I just recall print preview using some sort of listener setup to block events...
Ah, actually print preview does not cancel events, you can verify this by shift-clicking a link in print preview. What it does try to do is to cancel scripts, but the method it uses doesn't work with frames.
Er.... That sucks. It should really be cancelling all events with a capturing chrome listener, if possible... :(
Yes, but you have to be careful not to cancel scrollbar events...
Right. That can be done by examining the target, I would think....
What about scrollbars in <iframe>/<textarea>/<select>s?
You check not just the target but also the parent... It really shouldn't be hard to distinguish them. iframes/Textareas/selects shouldn't be scrollable in print preview anyway (are they now?)
Ah, no, they're not. So one problem solved...
Created attachment 149644 [details] [diff] [review] Lame hack Hmm... this does stop the crash, and scrollbars still work...
Hmm.. So is nsPrintPreviewListener basically unused?
Created attachment 149656 [details] [diff] [review] Better patch Just uncommenting the code to add the listeners wasn't sufficient, presumably because the listeners were bubbling and not capturing. Let me know if there's a better way to set all these capturing listeners.
Comment on attachment 149656 [details] [diff] [review] Better patch Hmm, this is a bit fragile given that as support for new input events is added (i.e. DOM Level 3 Events' Text event etc) we'll potentially need to add them to this list. But I can't really come up with a better solution to this right now... r=jst
Comment on attachment 149656 [details] [diff] [review] Better patch Can't find a better sr, sorry.
Comment on attachment 149656 [details] [diff] [review] Better patch sr=jst
Fix checked in.
So now scrollbars don't react to click events - bug 245751.
Verified FIXED using the http://bugzilla.mozilla.org/attachment.cgi?id=149572&action=view testcase in Print Preview on Windows XP (build 2004-08-16-13).
Although I cannot reproduce the crash, this patch prevents users from right clicking / middle clicking / etc. in the print previewed website, correct? If so, this is something we might want for the Aviary Branch (along with the fix for bug 245751). Nominating for Aviary 1.0.