JavaScript error: chrome://messenger/content/systemIntegrationDialog.xul, line 1: ReferenceError: gSystemIntegrationDialog is not defined

RESOLVED FIXED in Thunderbird 36.0

Status

defect
--
critical
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ishikawa, Assigned: ishikawa)

Tracking

({regression})

Trunk
Thunderbird 36.0
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird34+ fixed, thunderbird35+ fixed, thunderbird36 fixed, thunderbird_esr3134+ fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

A missing "}" near the end of JS file seems to have
caused the failure of parsing, and
I could not get DEBUG BUILD of C-C TB to run past the
OS integration dialog. Even if I hit "Skip Integration" nothing happens and I noticed the following
error message on the consoel where I invoked 
the binary.
 [...]
++DOMWINDOW == 30 (0x4fa6b20) [pid = 10672] [serial = 30] [outer = 0x4fa48f0]
JavaScript error: chrome://messenger/content/systemIntegrationDialog.js, line 138: SyntaxError: missing } after property list
JavaScript error: chrome://messenger/content/systemIntegrationDialog.xul, line 1: ReferenceError: gSystemIntegrationDialog is not defined
JavaScript error: chrome://global/content/bindings/dialog.xml line 370 > Function, line 1: ReferenceError: gSystemIntegrationDialog is not defined
JavaScript error: chrome://global/content/bindings/dialog.xml line 370 > Function, line 1: ReferenceError: gSystemIntegrationDialog is not defined
JavaScript strict warning: resource:///modules/gloda/indexer.js, line 1223: ReferenceError: reference to undefined property this._actualWorker
--DOMWINDOW == 29 (0x46fc220) [pid = 10672] [serial = 17] [outer = (nil)] [url = about:blank]
--DOMWINDOW == 28 (0x4715840) [pid = 10672] [serial = 18] [outer = (nil)] [url = about:blank]
--DOMWINDOW == 27 (0x2a265a0) [pid = 10672] [serial = 15] [outer = (nil)] [url = about:blank]
--DOMWINDOW == 26 (0x448aec0) [pid = 10672] [serial = 16] [outer = (nil)] [url = about:blank]
--DOMWINDOW == 25 (0x28d82e0) [pid = 10672] [serial = 2] [outer = (nil)] [url = about:blank]
--DOMWINDOW == 24 (0x476cb20) [pid = 10672] [serial = 19] [outer = (nil)] [url = about:blank]
JavaScript error: chrome://global/content/bindings/dialog.xml line 370 > Function, line 1: ReferenceError: gSystemIntegrationDialog is not defined
^C

Patch will follow.

TIA
Posted patch Add a closing brace "}". (obsolete) — Splinter Review
Here is a band-aid patch.

But I am not sure of the position of the brace.
I simply searched for the place where the proper matching occurred, but
the location looks suspicious.

http://mxr.mozilla.org/comm-central/source/mail/base/content/systemIntegrationDialog.js#99

 99   if (searchIntegPossible) {
100       this.SearchIntegration.firstRunDone = true;
101 
102     // If the "skip integration" button was used do not set any defaults
103     // and close the dialog.
104     if (!aSetAsDefault) {
105       // Disable search integration in this case.
106       if (searchIntegPossible)
107         this.SearchIntegration.prefEnabled = false;
108 
109       return true;
110  

It is possible that the if on line 99 may have been
intended to have a brace on line 101.

TIA
Attachment #8507396 - Flags: review?(standard8)
Comment on attachment 8507396 [details] [diff] [review]
Add a closing brace "}".

Looking at http://hg.mozilla.org/comm-central/rev/2f79b0b939ed, I think the } should go after:

let searchIntegPossible = !this._searchCheckbox.hidden;
  if (searchIntegPossible) {
    this.SearchIntegration.firstRunDone = true;

(approx line 100)
Attachment #8507396 - Flags: review?(standard8) → review-
(In reply to Mark Banner (:standard8) (away until 23rd Oct) from comment #2)
> Comment on attachment 8507396 [details] [diff] [review]
> Add a closing brace "}".
> 
> Looking at http://hg.mozilla.org/comm-central/rev/2f79b0b939ed, I think the
> } should go after:
> 
> let searchIntegPossible = !this._searchCheckbox.hidden;
>   if (searchIntegPossible) {
>     this.SearchIntegration.firstRunDone = true;
> 
> (approx line 100)

This was what I thought after the initial band-aid patch.
Thank you for confirming my suspicion.
Patch attached.
Assignee: nobody → ishikawa
Attachment #8507396 - Attachment is obsolete: true
Attachment #8507881 - Flags: review?(standard8)
Attachment #8507881 - Flags: review?(standard8) → review+
Thank you. I put in "checkin-needed keyword".
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/e4178e4b5c9a
Status: NEW → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 36.0
Yes, it breaks the integration dialog completely.
I would have fixed it too but was at the summit these days, where we found about the bug.
I would have removed the opening brace, but your fix works too.
This was caused by bug 1036592. I wonder how that got through 3 people (including me) :)

Thanks for fixing it so quickly.
Blocks: 1036592
Keywords: regression
OS: Linux → All
Version: unspecified → Trunk
(In reply to :aceman from comment #6)
> Yes, it breaks the integration dialog completely.
> I would have fixed it too but was at the summit these days, where we found
> about the bug.
> I would have removed the opening brace, but your fix works too.
> This was caused by bug 1036592. I wonder how that got through 3 people
> (including me) :)
> 
> Thanks for fixing it so quickly.

I was vaguely aware of the summit since Jcranmer also mentioned it. I hope that have been a very useful event.

Strange thing about this bug is that |make mozmill| did not complain about this very loudly. (or maybe the failing tests were due to this? I am doubtful.)
It is as if scripting worked even though this issue may have existed.
But I am glad that C-C tree would be in a good shape shortly.
Comment on attachment 8507881 [details] [diff] [review]
Add a closing brace "}", hopefully at the right place.

[Approval Request Comment]
This is part of bug 1036592 so must be checked in simultaneously.
Attachment #8507881 - Flags: approval-comm-esr31?
Comment on attachment 8507881 [details] [diff] [review]
Add a closing brace "}", hopefully at the right place.

[Approval Request Comment]
Regression caused by (bug #): bug 1036592 
User impact if declined: Unable to dismiss integration dialog
Testing completed (on c-c, etc.): 
Risk to taking this patch (and alternatives if risky): we took bug 1036592 for 34 and 35, so this needs to land there as well
Attachment #8507881 - Flags: approval-comm-beta?
Attachment #8507881 - Flags: approval-comm-aurora?
Comment on attachment 8507881 [details] [diff] [review]
Add a closing brace "}", hopefully at the right place.

Please land this on aurora/beta asap.
Attachment #8507881 - Flags: approval-comm-beta?
Attachment #8507881 - Flags: approval-comm-beta+
Attachment #8507881 - Flags: approval-comm-aurora?
Attachment #8507881 - Flags: approval-comm-aurora+
(I'm not in position to land these atm.)
Duplicate of this bug: 1097712
Attachment #8507881 - Flags: approval-comm-esr31? → approval-comm-esr31+
You need to log in before you can comment on or make changes to this bug.