543205 - [SM 2.1] Print Preview never shows up (tabbrowser.loadOneTab is not a function)

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Jens Hatlak (:InvisibleSmiley)]

Print Preview in SeaMonkey trunk builds never shows up due to a change in the Toolkit JS implementation.

a) First Invocation:

1. A small window opens.
Window title: Print Preview
Window contents:
Title: <website title>
Progress: Preparing...

2. Error Console output:
Error: tabbrowser.loadOneTab is not a function
Source File: chrome://global/content/printUtils.js
Line: 216

b) Second and Later Invocations:

Error Console output:
Error: aWindow is not defined
Source File: chrome://global/content/printUtils.js
Line: 107

Actual file is: /toolkit/components/printing/content/printUtils.js
Regression introduced by changeset: http://hg.mozilla.org/mozilla-central/rev/180966423a3c -> Bug 487667

Possible solution: Implement tabbrowser.loadOneTab in SM for compatibility

Comment 1

[Jens Hatlak (:InvisibleSmiley)]

Changing severity to major since it totally breaks the functionality.

Comment 2

[[e-mail]]

Note that loadOneTab is overkill for adding a tab; addTab suffices. But I also noticed that our getPPBrowser() doesn't actually return the tabbed browser; this didn't matter before because it was only being used to temporarily collapse the element apparently to fix some layout issue. The third issue is that addTab (or whatever) unhides the tabbar and of course our toggleAffectedChrome callback saves the browser state after the tab has been added, but restores it after it has been removed, so I ended up just commenting that code, since removing the tab should correctly restore the tabbar depending on preferences.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Yes yes, I'm fixing this.
I just need to get bug 544018 fixed first.
After that there is no need for loadOneTab nor addTab but
two other methods which make printutils.js pretty flexible.

Comment 4

[erpman1]

(In reply to comment #3)
> Yes yes, I'm fixing this.
> I just need to get bug 544018 fixed first.
> After that there is no need for loadOneTab nor addTab but
> two other methods which make printutils.js pretty flexible.

well how long will it take you to fix the print preview problem, Olli?
I'm also experiencing the problem with SM 2.1a1pre nightly build of 20100206 on my XP machine.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

This is pretty close to the FF version of the patch.

Comment 6

[[e-mail]]

Comment on attachment 425968 [details] [diff] [review]
patch

>-    theTabbrowser.setStripVisibilityTo(false);
Nit: you removed all the uses of this variable, but not the variable itself.

>+    getBrowser().mStrip.setAttribute("moz-collapsed", "true");
It's safe to use gBrowser here. (To see the irony, look at the line after the line indirectly referenced to by the previous comment.)

Comment 7

[[e-mail]]

Bah, I crashed with print preview open, and session store tried to restore :-(

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Well, that sounds like a different bug.
I doubt session store handles print preview properly.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Attachment: +comments

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

http://hg.mozilla.org/comm-central/rev/8b048e37dc57

Comment 11

[[e-mail]]

(In reply to comment #8)
> Well, that sounds like a different bug.
> I doubt session store handles print preview properly.
But that wasn't a problem before print preview opened tabs...

Comment 12

[[e-mail]]

Actually you could have used gBrowser throughout the patch, but never mind.