267422 - Print Preview window is partly overlayed by background and is missing scrollbars

Status: VERIFIED FIXED

(Toolkit :: Printing, defect)

Description

[Adam Hauner]

2004110104/trunk/W2K

Repro:
1. open http://www.pcmag.com/article2/0,1759,1937,00.asp
2. open Print Preview window
3. in Page Setup switch Print Backgrounds

Actual:
Part of Print Preview window is overlayed by XUL default background.

Netscape 7.2 (based on 1.7.2) doesn't contain this bug.

Comment 1

[Adam Hauner]

Attachment: screenshot of bug

Comment 2

[Boris Zbarsky [:bzbarsky]]

Can you narrow down the regression window here?

Comment 3

[Adam Hauner]

Boris: 2004101107 - okay, 2004101206 - bad

Comment 4

[Boris Zbarsky [:bzbarsky]]

Sounds like widget change caching fallout....  I assume this is still happening
in a current trunk build?  (I'd test, but mine crashes when I try to
print-preview that page).

Comment 5

[Adam Hauner]

Boris: I found that page by crash bug 252364 (maybe you should reopen it), bug
265617 is also related.

Comment 6

[Boris Zbarsky [:bzbarsky]]

No, I see a totally different crash from that one, and only in my opt build....
 <sigh>

Comment 7

[Adam Hauner]

Retested with 2004111404/trunk/W2K and problem is still here.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

My GTK1 build doesn't crash, and doesn't display the bug because there's no Page
Setup controls in Print Preview in GTK builds...

Comment 9

[Paul Gregg]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a5) Gecko/20041122
Just noticed this in 1.8a5, and found easier reproducing steps...

I can get this bug to occur on any page, and just need to change the scale from
the toolbar to see it.  Resizing the window makes the obtruding chrome return to
normal.

Comment 10

[Stefan Borggraefe]

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

Comment 11

[Doug Halamay]

I can confirm this bug still exists in the most recent nightly...this is a
pretty nasty one.  It occurs with any page that I do a preview for and then
either try to change the layout (portrait/landscape) or scale.  In fact,
attempting to change the layout from portrait to landscape, or vice versa, seems
to do nothing at all, but I'm not sure if that is this bug or not.  This is with
WinXP SP2.

Mozilla/5.0 (Windows; compatible; U; Windows NT 5.1; en-US; rv:1.8b2)
Gecko/20050325 Firefox/1.0+

Comment 12

[kevinn]

Attachment: screenshot of bug in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050417 Firefox/1.0+

Nasty bug that's still in latest nightly builds.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050417
Firefox/1.0+

Comment 13

[José Jeria]

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

Comment 14

[José Jeria]

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

Comment 15

[José Jeria]

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

Comment 16

[Jean-Michel Reghem]

I confirm the bug ...
When you rescale, it just mask a part of the page, and then, resizing do nothing
but a vertical scroll came ...
And, for some page, it drives to a crash as bug #299107

Comment 17

[Bryan]

Confirmed this is still an issue and makes Print Preview virtually unusable for
me.  Requesting blocking on 1.8b4/1.1.

BUILD: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b3) Gecko/20050708
Firefox/1.0+ ID:2005070808

Comment 18

[Doug Halamay]

Is 1.1 really going to go out with this bug still not fixed?  This is a major UI
regression, which causes the print preview window to be pretty much useless when
changing anything from page scaling to portrait/landscape in the UI.

Comment 19

[Doug Halamay]

Is 1.1 really going to go out with this bug still not fixed?  This is a major UI
regression, which causes the print preview window to be pretty much useless when
changing anything from page scaling to portrait/landscape in the UI.

Comment 20

[Jean-Michel Reghem]

Yes, it is really annoying ... the functionnality donn't work ...
Maybe it is too late to fix as Deer Park Alpha 2 is gone ... but it is a pity

Comment 21

[Jaime Mitchell (use [e-mail] for email)]

(In reply to comment #19)
> Is 1.1 really going to go out with this bug still not fixed?  This is a major UI
> regression, which causes the print preview window to be pretty much useless when
> changing anything from page scaling to portrait/landscape in the UI.

Hopefully not, the reason the blocking aviary1.1 flag was removed is that this
needs to be fixed before 1.8b4 if it is to make firefox 1.1. (see some of the
other bugs in which simular flag changes were made). Hopefully this will be
granted blocking 1.8b4 shortly.

Comment 22

[Jaime Mitchell (use [e-mail] for email)]

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

Comment 23

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Windows windows windows!

Comment 24

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We've got to fix this before release, but I think it's WIndows only.

Comment 25

[Doug Halamay]

Any reason this was set blocking-aviary1.5- ?  As I said before, this is a clear
regression that I doubt you would want in a release.  And roc said it needed to
be fixed, also!

Comment 26

[José Jeria]

There is no overlaying part in the print preview anymore. Now the scale dropdown
does simply nothing.

I get the following error each time i select a value:
Error: uncaught exception: [Exception... "Component returned failure code:
0x80004005 (NS_ERROR_FAILURE)
[nsIPrintSettingsService.savePrintSettingsToPrefs]"  nsresult: "0x80004005
(NS_ERROR_FAILURE)"  location: "JS frame ::
chrome://global/content/printUtils.js :: anonymous :: line 86"  data: no]

Comment 27

[Frank Wein [:mcsmurf]]

José: i think this is another regression, file new bug for this (or maybe use
Bug 302495 for that)?

Comment 28

[José Jeria]

(In reply to comment #27)
> José: i think this is another regression, file new bug for this (or maybe use
> Bug 302495 for that)?

Yes, you are right. That bug is what I see

Comment 29

[Adam Guthrie]

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

Comment 30

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Is this bug still happening?

Comment 31

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

Yup, it's very visible on build Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
rv:1.9a1) Gecko/20050822 Firefox/1.6a1

Comment 32

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Asa/mconnor, we *have* to fix this for FF1.5. It's just incredibly bad. Look at
the screenshot.

Comment 33

[Wayne Mery (:wsmwk)]

attempting to reproduce this with
http://www.pcmag.com/article2/0,1759,1937,00.asp on Mozilla/5.0 (Windows; U;
Windows NT 5.1; en-US; rv:1.9a1) Gecko/20050816

FWIW this link + print preview doesn't behave the same as on my XP home system
with FF 20050808. 

I eventually crashed TB8635705E stack signature 0x6f72504c be3ae473
I haven't found a bug that matches this signature.

Comment 34

[José Jeria]

I agree with comment 32, this is a serious UI regression. It's a badly broken
feature that should not be available in it this state

Comment 35

[Mike Schroepfer]

Roc - Are you able to spend any cycles on this?

Comment 36

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I would, but I don't have a Windows setup.

Comment 37

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

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

Comment 38

[Thomas Rutter]

Attachment: Screenshot of problem with gray area on both right and bottom

Comment 39

[Felix Miata]

Same problem in OS/2 trunk. Resize fixes it. In addition, no matter what scale I
set, it prints exactly the same thing. Only by changing my font-size pref can I
make it print larger or smaller.

Comment 40

[Thomas Rutter]

(In reply to comment #39)
> In addition, no matter what scale I
> set, it prints exactly the same thing. Only by changing my font-size pref can I
> make it print larger or smaller.

That issue is addressed in bug 302495.

Comment 41

[Wayne Mery (:wsmwk)]

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

Comment 42

[Adam Guthrie]

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

Comment 43

[Adam Guthrie]

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

Comment 44

[Andre Sihera]

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

Comment 45

[Asa Dotzler [:asa]]

David, can you help us with this? It's likely to fall of the blocker list if we
don't get movement on it ASAP.

Comment 46

[Frank Yan (:fryn)]

(In reply to comment #45)
> David, can you help us with this? It's likely to fall of the blocker list if 
we
> don't get movement on it ASAP.

yeah, this is a major bug and definitely should be fixed.

Comment 47

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I had a look at this. It is a real showstopper IMHO.

I wonder whether Boris's fix in 301411 might have helped here. I'm making a
build to find out.

Comment 48

[Robert O'Callahan (:roc) (email my personal email if necessary)]

stephend told me on IRC that this bug is still present after that fix.

We need someone with a Windows box on their desk to work on this. Whoever
volunteers or is volunteered, I can help walk through with them via IRC.

Comment 49

[Asa Dotzler [:asa]]

vlad or pav, can you guys help us out here?

Comment 50

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

I've been looking, but not making much progress -- I'm not familiar with
printing at all.  However, it seems that this is more of a cosmetic issue (but a
bad one); the real issue is that print preview is almost entirely useless.  Most
of the controls don't do anything -- with the patch in 302495, the various
buttons/dropdowns (scale, portrait/landscape) retain their state when clicked
now, but they still have no effect on the final page.  Fixing this particular
gray-area issue will still leave print preview mostly broken.

Can roc or someone who knows printing but doesn't have time to look into it (or
a windows box) leave a comment suggesting possible directions to look?

Comment 51

[Asa Dotzler [:asa]]

radical suggestion, I'm sure, but perhaps we should just remove this broken
feature. The usability of the feature when it was "working" was questionable at
best.

Comment 52

[Michael]

(In reply to comment #51)
> radical suggestion, I'm sure, but perhaps we should just remove this broken
> feature. The usability of the feature when it was "working" was questionable at
> best.

Remove print preview or remove the print preview toolbar for changing print
settings? I can't say for standard web users how useful either is, but for web
developers like myself it's excellent in seeing how pages will most likely print
without actually printing them. Changing between landscape and portrait in the
print preview window is a tool I use quite often.

Comment 53

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Removing print preview is the wrong thing to do; I think I can get a fix for
this very soon (also a fix for 302495, which I already have fixed).

Comment 54

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: print-preview-fix.patch

This is not an ideal fix.  However, it does work.  This also fixes bug 302495.

Whenever a print setting is changed, to get the right reflow mechanics, this
forces an exit/enter of print preview mode; in firefox, this causes the
menus/toolbars to flash up on the screen for a second.	It's ugly, but it's
better than having broken print preview, and things get displayed correctly at
that point.   Print Preview UI needs to be reworked on the trunk (taking over
the window is a pretty bad idea).

kInitSaveNativeData isn't used/handled anywhere in the code, so
PrintUtils.savePrintSettings() wasn't saving anything useful.  Got rid of that
function and had the toolbar save expliticlty what it needs to save.  We also
don't clear out the enter/exit pp callbacks once we've called them once; this
/shouldn't/ break anything, but I'm not sure if tbird or something has some
complex behaviour in here.

Setting r? mscott/bz, since I have no idea who can review this quickly..

Comment 55

[Scott MacGregor]

Vlad, I believe bz is gone until tomorrow evening. Maybe we can use mconnor here
instead.

Comment 56

[Scott MacGregor]

Comment on attachment 198372 [details] [diff] [review]
print-preview-fix.patch

switching to mike for the 2nd review since bz is gone.

I've been testing with this patch and can confirm that it makes things better
for me. 

Do you need the double semi colons here?

+	 <body><![CDATA[
+	   ;;

Comment 57

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Whoops, nope; the semicolonos can go.  In further thinking about this I think we
can get away with just hiding/showing the print preview toolbar -- which would
be much less intrusive.  I might have an updated patch in a second.

Comment 58

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: print-preview-fix.patch (version 2)

Much better patch.  Just hide/show the print preview toolbar; it doesn't even
look horrible doing it this way, and there's very little risk since we don't
change the semantics of the preview callbacks.

Comment 59

[Boris Zbarsky [:bzbarsky]]

I wouldn't be able to review this anyway -- I don't know this code.

Neil, xpfe might need these changes...

Comment 60

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: new patch, what's really going in

Argh. bug 302495 and bug 275387 are identical; the patch for 275387 landed
while I was working on this today.  I'm going to change the way that it's
handled though, to remove some unnecessary calls.

In any case, this is what's really going in to both branch and trunk.  Also
fixes 302495 and 275387.

Comment 61

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Checked in on branch and trunk.

Comment 62

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

This bug was filed against Mozilla before it became SeaMonkey, so it's not
really fixed until someone lands a fix for SeaMonkey, right?

Comment 63

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Once Seamonkey switches to using toolkit for print preview, it'll get the fixes too!

Comment 64

[Adam Hauner]

I really appreciate your fixes (after 11 months), but I created this bug for
Mozilla Suite and AFAIK isn't fixed in SeaMonkey. REOPEN.

Comment 65

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Then this is the wrong component, if it's intended to be a Seamonkey bug; please
file a new bug against the Mozilla Application Suite.  (Though, Core -> Print
Preview isn't useful either; if anything this should've become Toolkit ->
Printing.)  However, this bug is fixed for the product that is shipping.

Comment 66

[Adam Hauner]

Grrr. This bug was filled in correct product Mozilla and its Print Preview. But
then were Bugzilla components reorganisation and Print Preview was moved to
Core. Moving this bug into Toolkit.
 
So I filled this bug for Mozilla/SeaMonkey for second time -> bug 311028.

Comment 67

[Bryan]

The root cause of this issue appears to screw with "View-> Text Size".  If you
lower the text size on a particular page and then click on File-> Print Preview
the text size is reverted back to the default size.  ARGH!!!  It sometimes
affects  every FF window where you've modified the text size via "View-> Text Size".

~B

Comment 68

[Adam Guthrie]

(In reply to comment #67)
> The root cause of this issue appears to screw with "View-> Text Size".  If you
> lower the text size on a particular page and then click on File-> Print Preview
> the text size is reverted back to the default size.

That's bug 172520.