407080 - Print preview can happen while the page is loading

Status: VERIFIED FIXED

(Core :: Print Preview, defect, P2)

Description

[Martijn Wargers (dead)]

Normally you get a dialog that says that the page is loading, you can't do print preview right now.
But in current trunk build that's not happening anymore, and you can do print preview while the page is loading.

That seems nice, but it has the bad effect that after closing print preview the page doesn't show up.

I haven't looked for a regression range yet.

Comment 1

[Martijn Wargers (dead)]

This regressed between 2007-02-22 and 2007-02-23:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-02-22+04&maxdate=2007-02-23+09&cvsroot=%2Fcvsroot
This is a regression from bug 370966.

The patch in that bug removed the NS_ERROR_GFX_PRINTER_DOC_IS_BUSY_PP error that apparently handled this.

Apparently, it is possible to print while a page is loading (even on branch), so I'm removing that part from the summary.

Comment 2

[Damon Sicore (:damons)]

+'ing.  P5.

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: patch v1

This patch just re-adds a chunk of nsPrintEngine::PrintPreview that was removed in bug 370966 (during the mergin of Print/ Print-Preview code.)

From doing some preliminary testing, it seems to work as expected.

Comment 4

[Daniel Holbert [:dholbert]]

(In reply to comment #3)
> in bug 370966 (during the mergin of Print/ Print-Preview code.)

s/mergin/merging/

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: patch v2 (improved comments)

Comment 6

[Daniel Holbert [:dholbert]]

For reference, this code comes straight from nsPrintEngine::PrintPreview, just before the landing of bug 370966's patch.

http://tinyurl.com/yqt4ml
(line 761 - 775)

Comment 7

[Daniel Holbert [:dholbert]]

patch v2 landed

Checking in nsPrintEngine.cpp;
/cvsroot/mozilla/layout/printing/nsPrintEngine.cpp,v  <--  nsPrintEngine.cpp
new revision: 1.165; previous revision: 1.164
done

Comment 8

[Andrew Schultz]

This busted mochitests.  test_bug396024.html invokes print preview and triggers the dialog here.

Comment 9

[Daniel Holbert [:dholbert]]

D'oh -- thanks for the heads-up.  Backed out, & will investigate.  (I'm guessing that the mochitest box just isn't giving the page enough time to load before calling PrintPreview on it.)

Comment 10

[Daniel Holbert [:dholbert]]

Reopening until I can re-land with mochitest fix.

Comment 11

[Martijn Wargers (dead)]

Which machine was busted? Afaik, there is no printer installed on any of the tinderboxes, see bug 406063, so it shouldn't be able to fail on that.
However, test_bug396024.html will indeed fail when you have printers installed with this patch. But I think the test is flawed here. It needs to be rewritten.

Comment 12

[Daniel Holbert [:dholbert]]

I'm pretty sure that both of these were broken:
   Linux qm-centos5-01 dep unit test
   MacOSX Darwin 8.8.4 qm-xserve01 dep unit test

Not 100% sure about the mac, though.

Comment 13

[Andrew Schultz]

This was busted on the FF Linux box and also my own box.  There's always a print-to-file printer available on Linux (postscript and now PDF).  The FF linux log is at

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1204592581.1204601771.24126.gz

Comment 14

[Daniel Holbert [:dholbert]]

Attachment: patch v2a (disabled failing mochitest) (checked-in)

Patch as-landed, with broken mochitest disabled, per conversation with Martijn and roc on IRC.  (Martijn's going to fix the mochitest at some point.)

Checking in base/tests/Makefile.in;
/cvsroot/mozilla/layout/base/tests/Makefile.in,v  <--  Makefile.in
new revision: 1.60; previous revision: 1.59
done
Checking in printing/nsPrintEngine.cpp;
/cvsroot/mozilla/layout/printing/nsPrintEngine.cpp,v  <--  nsPrintEngine.cpp
new revision: 1.167; previous revision: 1.166
done

Comment 15

[Martijn Wargers (dead)]

Ok, thanks, verified fixed in current trunk build.
I still find it sort of odd that you can print while the page is loading, but I'm not really sure if you can call it a bug or not.

But I think those printer error dialogs can be really annoying and if someone would want to make an automated testcase for this bug, then you would need a way to disable those error dialogs, so I filed bug 422949 for it.

Comment 16

[Martijn Wargers (dead)]

Btw, I have fixed the broken mochitest from bug 396024 now.

Comment 17

[Serge Gautherie (:sgautherie)]

(In reply to comment #16)
> Btw, I have fixed the broken mochitest from bug 396024 now.

http://hg.mozilla.org/mozilla-central/rev/3093b487202a