102152 - javascript strict warnings in pref-mailnewsOverlay.js

Status: RESOLVED DUPLICATE of bug 121369

(MailNews Core :: Simple MAPI, defect)

Description

[Henrik Gemal]

just starting mailnews gives me:

Warning: reference to undefined property 
Components.classes['@mozilla.org/mapiregistry;1']
Source File: chrome://messenger/content/pref-mailnewsOverlay.js
Line: 27

Comment 1

[Henrik Gemal]

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

Comment 2

[Srilatha Moturi]

Attachment: (Av1) patch v1

Comment 3

[Srilatha Moturi]

Attachment: (Av1a) patch

Comment 4

[Srilatha Moturi]

Reassigning this bug to myself.
Seth, can you review the patch

Comment 5

[Stephen Walker]

Henrik, are you still seeing this bug?

Comment 6

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b) Gecko/20050212] (nightly) (W98SE)

{{
Warning: redeclaration of var mailnewsEnableNews
Source File: chrome://messenger-mapi/content/pref-mailnewsOverlay.js
Line: 54, Column: 12
Source Code:
        var mailnewsEnableNews = document.getElementById("mailnewsEnableNews"); 
}}

Comment 7

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1) <pref-mailnewsOverlay.js>

I wonder if
{{
    var mailnewsEnableMapi = document.getElementById("mailnewsEnableMapi");
    var mailnewsEnableNews = document.getElementById("mailnewsEnableNews");
}}
could be made global to the file !?

Open question:
Is
{{
+	 var mapiRegistryProgID = "@mozilla.org/mapiregistry;1" 
+	 if (mapiRegistryProgID in Components.classes) {
}}
from previous uploaded patch "still" needed ?
Opening MailNews doesn't show the warning on official releases,
but maybe it could on some customized builds ??

Comment 8

[[e-mail]]

Comment on attachment 174336 [details] [diff] [review]
(Bv1) <pref-mailnewsOverlay.js>

>+        mailnewsEnableMapi.setAttribute("checked",
>+          parent.mapiPref.isDefaultMailClient ? "true" : "false");

>+          mailnewsEnableNews.setAttribute("checked",
>+            parent.mapiPref.isDefaultNewsClient ? "true" : "false");
These should just use the .checked property although setAttribute already uses
the obvious string conversion. r=me with this fixed.

Comment 9

[Serge Gautherie (:sgautherie)]

Comment on attachment 54770 [details] [diff] [review]
(Av1a) patch

Neil replied me
{{
1) No, you can't use document.getElementById globally.
2) In theory that should never happen. The "@mozilla.org/mapiregistry;1" 
component and the pref-mailnewsOverlay.js are part of the same module, 
so Windows computers should have them both and other computers will have 
neither. But maybe the reporter had some corruption somewhere.
}}

Then, dropping initial report (no reply since 2002-05-11),
and morphing to the current warning (duped bug 282242).

Comment 10

[Serge Gautherie (:sgautherie)]

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

Comment 11

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1) <pref-mailnewsOverlay.js>

Bv1, with comment 8 suggestion(s),
and some more.

I wonder if |function mailnewsOverlayInit()|
could use something like
{{
	    if (prefBranch && prefBranch.prefIsLocked("defaultNewsClient")) {
		mapiRegistry.isDefaultNewsClient =
prefBranch.getBoolPref("defaultNewsClient") ;
		mailnewsEnableNews.disabled = true;
}}
or a comment so say why there is not ??

Comment 12

[[e-mail]]

Comment on attachment 174416 [details] [diff] [review]
(Bv1) <pref-mailnewsOverlay.js>

The inability to lock the news preference does look like an oversight.

Comment 13

[Serge Gautherie (:sgautherie)]

Comment on attachment 174416 [details] [diff] [review]
(Bv1) <pref-mailnewsOverlay.js>

(In reply to comment #12)
> The inability to lock the news preference does look like an oversight.

Is fixing this as simple as adding "the" _js_ lines I suggested in my previous
comment, or could you point me to a full "example" ?
(I'll file a new bug if needed...)

Comment 14

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 174416 [details] [diff] [review]
(Bv1) <pref-mailnewsOverlay.js>

I think your proposed changes to lock those prefs look fine.  sr=dmose with or
without those changes.

Comment 15

[Serge Gautherie (:sgautherie)]

(In reply to comment #14)
> (From update of attachment 174416 [details] [diff] [review] [edit])
> I think your proposed changes to lock those prefs look fine.  sr=dmose with or
> without those changes.

Noted: I'll prepare an additional patch based on comment 11, in the future...

Comment 16

[Serge Gautherie (:sgautherie)]

Comment on attachment 174416 [details] [diff] [review]
(Bv1) <pref-mailnewsOverlay.js>

'approval1.8b2=?':
Trivial U.I. code cleanup, no risk.
This should be MailNews specific !?

Comment 17

[Asa Dotzler [:asa]]

Comment on attachment 174416 [details] [diff] [review]
(Bv1) <pref-mailnewsOverlay.js>

try again when we're not so close to a release.

Comment 18

[Serge Gautherie (:sgautherie)]

Comment on attachment 174416 [details] [diff] [review]
(Bv1) <pref-mailnewsOverlay.js>

Obsoleting: this patch never got checked in :-(
(See bug 121369, which bitrotted/superseded this patch.)

Comment 19

[Serge Gautherie (:sgautherie)]

Bug 121369 even added comment 11.