Closed Bug 1402671 Opened 2 years ago Closed 2 years ago

undeclared variable debug in offlineStartup.js

Categories

(MailNews Core :: Backend, defect)

defect
Not set

Tracking

(thunderbird57 fixed, thunderbird58 fixed, seamonkey2.54 fixed, seamonkey2.55 fixed)

RESOLVED FIXED
Thunderbird 58.0
Tracking Status
thunderbird57 --- fixed
thunderbird58 --- fixed
seamonkey2.54 --- fixed
seamonkey2.55 --- fixed

People

(Reporter: frg, Assigned: frg)

References

Details

Attachments

(1 file, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #1401897 +++

Variable debug is never declared in mailnews\extensions\offline-startup\js\offlineStartup.js

See Bug 1401897 Comment 1

> JavaScript error: file:///home/dme/comm-central/obj-sm-release/dist/bin
> /components/offlineStartup.js, line 165: ReferenceError: assignment to 
> undeclared variable debug
Attached patch 1402671-debug.patch (obsolete) — Splinter Review
[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: 
Testing completed (on c-c, etc.): 
Risk to taking this patch (and alternatives if risky):

I didn't test it yet. 

Renamed debug because I find such generic variable names confusing.
Attachment #8911537 - Flags: review?(jorgk)
Attachment #8911537 - Flags: approval-comm-beta?
Sorry hit Enter too fast.

[Approval Request Comment]
Regression caused by (bug #): 1394556
User impact if declined: broken functionality
Testing completed (on c-c, etc.): 
Risk to taking this patch (and alternatives if risky): none already broken.
Comment on attachment 8911537 [details] [diff] [review]
1402671-debug.patch

So all you did was to move the definition to the top and rename it, right? r+ if you promise that it works now ;-)

> I didn't test it yet.
... in and approval request is really not the right thing to say. Usually we approve after testing and landing on trunk.

BTW, using offline mode from the profile Profile Manager on Daily doesn't show anything in the error console.
Attachment #8911537 - Flags: review?(jorgk)
Attachment #8911537 - Flags: review+
Attachment #8911537 - Flags: approval-comm-beta?
Attachment #8911537 - Flags: approval-comm-beta+
Comment on attachment 8911537 [details] [diff] [review]
1402671-debug.patch

Actually, looking at it again, all that was missing was "var debug;" somewhere.

You have:
 var nsOfflineStartup =
 {
   onProfileStartup: function()
   {
    debug("onProfileStartup");
...

and later:
if (!kDebug)	
  debug = function(m) {}; ######

So surely 'debug' is set before ever accessed. In bug 1401897 you're showing the real error:
ReferenceError: assignment to undeclared variable debug
and that comes from the line I marked. Your solution is more elaborate, but a one-liner would have fixed it.
Comment on attachment 8911537 [details] [diff] [review]
1402671-debug.patch

Actually, looking at this again, this should be called 'kDebugLog' since k is for konstants ;-) Please call it gDebugLog. It's OK to move the assignment up, so it's clearly visible.

So r+ for a patch with s/kDebugLog/gDebugLog/ and the declaration of the variable after gOfflineStartupMode.
Attachment #8911537 - Flags: review+
Attachment #8911537 - Flags: approval-comm-beta+
Now actually tested :) r+ from jorgk carried over after reviewer comments fixed.

[Approval Request Comment]
Regression caused by (bug #): 1394556
User impact if declined: offline-startup broken.
Testing completed (on c-c, etc.): c-c
Risk to taking this patch (and alternatives if risky): none already broken.

Do you want to check it in yourself?
Attachment #8911537 - Attachment is obsolete: true
Attachment #8911586 - Flags: review+
Attachment #8911586 - Flags: approval-comm-beta?
Btw. I saw the error during normal startup in SeaMonkey 2.55
(In reply to Frank-Rainer Grahl (:frg) from comment #6)
> Now actually tested :) r+ from jorgk carried over after reviewer comments
> fixed.
Good, thanks.

> Do you want to check it in yourself?
I trust you, I don't see the error in the first place.
Keywords: checkin-needed
Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/f17b4b49ffa6
Declare and rename debug function in offlineStartup.js. r=jorgk
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 58.0
Attachment #8911586 - Flags: approval-comm-beta? → approval-comm-beta+
See Also: → 1404601
You need to log in before you can comment on or make changes to this bug.