191158 - javascript strict warnings in mailWindowOverlay.js, printing.js, and printPreviewBindings.xml

Status: RESOLVED FIXED

(Core :: Print Preview, defect)

Description

[Henrik Gemal]

If you right click in an empty message pane and select print preview

Warning: assignment to undeclared variable gPrintSettingsAreGlobal
Source File: chrome://communicator/content/printing.js
Line: 65

Warning: assignment to undeclared variable gSavePrintSettings
Source File: chrome://communicator/content/printing.js
Line: 66

Error: uriArray has no properties
Source File: chrome://messenger/content/msgPrintEngine.js
Line: 260

20030128

Comment 1

[Boris Zbarsky [:bzbarsky]]

please don't cc me on random ui bugs

Comment 2

[Steve Chapel]

I have a patch that fixes the warnings, but I could not reproduce the error. How
do I get an empty message pane that I can right-click in?

Comment 3

[Steve Chapel]

Attachment: (Av1) fix

I figured out how to get an empty message pane: first select a message, then
select another folder. This patch fixes the original warnings and error, plus
an additional warning that appeared after fixing those. With this patch,
selecting Print Preview on an empty message pane leads to no warnings or
errors, and selecting Print Preview on a non-empty message pane still works.

Selecting Print Preview on an empty message pane will now just briefly display
a Print Preview dialog and do nothing else. Probably the Print Preview item in
the context menu should be disabled in the case of an empty message pane.

Comment 4

[[e-mail]]

Yeah, that menuitem should be disabled unless a message is loaded.
I thought that there was already a bug for that, but I can't find it.
If there is, then this bug is invalid/duplicate/whatever.

Comment 5

[Steve Chapel]

It looks like bug 173074 is that bug. But this bug still is not invalid, because
three warnings are produced when selecting Print Preview on a non-empty message
pane. Changing summary to reflect warnings now produced.

Comment 6

[[e-mail]]

Strange, I get six warnings :-)

Comment 7

[Steve Chapel]

Attachment: (Av2) Fix for four warnings

This patch fixes the four warnings I currently get. If anyone can reproduce
more warnings, please list the steps and the warnings.

Comment 8

[Steve Chapel]

Attachment: (Av2b) Fix for five warnings

With this patch and the reviewed patch for bug 173074, I cannot generate any
warnings or errors with Print Preview in MailNews.

Comment 9

[[e-mail]]

Comment on attachment 130212 [details] [diff] [review]
(Av2b) Fix for five warnings

>@@ -399,7 +396,7 @@
>           var webBrowserPrint;  
>           var domWin;  
>           try {
>-            var ifreq = _content.QueryInterface(Components.interfaces.nsIInterfaceRequestor);
>+            ifreq = _content.QueryInterface(Components.interfaces.nsIInterfaceRequestor);
>             domWin = ifreq.QueryInterface(Components.interfaces.nsIDOMWindow);
>           } catch (e) {
>             // Pressing cancel is expressed as an NS_ERROR_ABORT return value,
Now unfortunately this code is a waste of time, as domWin == content  :-)
Had this code been necessary, the correct fix would have been to remove the var
ifreq; at line 398.

>@@ -39,6 +39,8 @@
>  * ***** END LICENSE BLOCK ***** */
> 
> var gPrintSettings = null;
>+var gPrintSettingsAreGlobal = null;
>+var gSavePrintSettings = null;
These are booleans. Find out whether they should default to true or false.

>@@ -40,6 +40,7 @@
> var gPrefs = Components.classes["@mozilla.org/preferences-service;1"].getService(Components.interfaces.nsIPrefBranch);
> var gPrintSettings = null;
> var gWindowReuse  = 0;
>+var printEngineWindow;
This variable is assigned to but never used... definitely no point making it a
global.

Comment 10

[Serge Gautherie (:sgautherie)]

Comment on attachment 130212 [details] [diff] [review]
(Av2b) Fix for five warnings


<printPreviewBindings.xml>
and
<printing.js>
were fixed in bug 228529...

<mailWindowOverlay.js>
Which fix is needed for |printEngineWindow|, variable declaration (and what use
?) or removal ?

Comment 11

[Wayne Mery (:wsmwk)]

I don't see the console messages.
Fixed?

Comment 12

[Serge Gautherie (:sgautherie)]

(In reply to comment #11)
> Fixed?

No:

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20050904 SeaMonkey/1.1a]
(nightly) (W98SE)

Try it on a message without body;
Note that the (strict) warning displays only the first time in the "session".

{{
Warning: assignment to undeclared variable printEngineWindow
Source File: chrome://messenger/content/mailWindowOverlay.js
Line: 1576
}}

Same code at
http://lxr.mozilla.org/mozilla/source/mail/base/content/mailWindowOverlay.js#1596

Variable removal seems appropriate.

David:
Would you r+sr a patch that would do just this ?

Comment 13

[:Cykesiopka]

Bug 462681 fixed the "assignment to undeclared variable printEngineWindow" issue.