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

Status: RESOLVED FIXED

(Thunderbird :: OS Integration, defect)

Description

[ISHIKAWA, Chiaki]

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

Comment 1

[ISHIKAWA, Chiaki]

Attachment: Add a closing brace "}".

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

Comment 2

[Mark Banner (: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)

Comment 3

[ISHIKAWA, Chiaki]

Attachment: Add a closing brace "}", hopefully at the right place.

(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.

Comment 4

[ISHIKAWA, Chiaki]

Thank you. I put in "checkin-needed keyword".

Comment 5

[Mark Banner (:standard8)]

https://hg.mozilla.org/comm-central/rev/e4178e4b5c9a

Comment 6

[:aceman]

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.

Comment 7

[ISHIKAWA, Chiaki]

(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 8

[:aceman]

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.

Comment 9

[Magnus Melin [:mkmelin]]

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

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 8507881 [details] [diff] [review]
Add a closing brace "}", hopefully at the right place.

Please land this on aurora/beta asap.

Comment 11

[Magnus Melin [:mkmelin]]

(I'm not in position to land these atm.)

Comment 12

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-aurora/rev/6d2d158a7f3e
https://hg.mozilla.org/releases/comm-beta/rev/8522dfd3262d

Comment 14

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-esr31/rev/5f8043f0ddc2