882901 - Error: "ReferenceError: tabContainer is not defined" in chrome://messenger/content/tabmail.xml

Status: RESOLVED FIXED

(Thunderbird :: Toolbars and Tabs, defect)

Description

[Stefan Sitter [:ssitter]]

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Thunderbird/24.0a1
BuildID: 20130612030727

I get the following error when closing a tab:

  Error: ReferenceError: tabContainer is not defined
  Source file: chrome://messenger/content/tabmail.xml
  Line: 2599

It seems this error is triggered or maybe just reported after I installed the Console² extension from https://addons.mozilla.org/thunderbird/addon/1815 in a fresh profile.

Comment 1

[:aceman]

Attachment: 882901.patch

This seems to be caused by bug 868969, where tabBrowser was changed to tabContainer without actually defining tabContainer as tabBrowser.tabContainer. At least that is the state in the current code. This seems to fix it. However I don't know what problem the error caused (if any) so can't test if the problem is gone now. But the error in the console is gone. The patch also fixes a "functions should only be defined on top level" warning.

Suyash, as you seem to be seeing the error message too, please check if the patch removes it.

The steps to reproduce this are just to open a new tab in TB (e.g. message in a tab) and then close it. No addons required (safe mode produces the error too).

Comment 2

[:aceman]

CCing guys involved in bug 868969 to check if the theory is correct.

Comment 3

[Suyash Agarwal (:sshagarwal)]

Comment on attachment 806860 [details] [diff] [review]
882901.patch

Awesome, yes I can confirm the error is gone now :)

Comment 4

[:aceman]

Comment on attachment 806860 [details] [diff] [review]
882901.patch

Review of attachment 806860 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 806860 [details] [diff] [review]
882901.patch

Review of attachment 806860 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/base/content/tabmail.xml
@@ +2558,5 @@
>      <handlers>
>        <handler event="click" button="0"><![CDATA[
> +        let bindingParent = document.getBindingParent(this);
> +        if (!bindingParent)
> +          return;

should fix the indention of the block below

Comment 6

[:aceman]

Yeah, I intentionally didn't reindent the whole block as it would make the changes invisible and would be churn that is normally unwanted. But if the reviewer insists on it I have no problem to do it.

Comment 7

[Magnus Melin [:mkmelin]]

In such cases it's often useful to fix it, but also submit an additional diff with whitespace changes ignored for review.

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 806860 [details] [diff] [review]
882901.patch

Review of attachment 806860 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good! Thanks aceman.

Comment 9

[:aceman]

mconley, should I now do the reindenting patch?

Comment 10

[Mike Conley (:mconley) (:⚙️)]

Yes - I should have been more clear: my r+ is conditional upon the fixes that Magnus asked for.

Comment 11

[:aceman]

Attachment: 882901.patch v2

OK, thanks.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/05ea0effa0a3

Comment 13

[:aceman]

Comment on attachment 813601 [details] [diff] [review]
882901.patch v2

[Approval Request Comment]
Regression caused by (bug #): bug 868969
User impact if declined: unknown
Testing completed (on c-c, etc.): c-c nightly
Risk to taking this patch (and alternatives if risky): unknown

This seems to work fine. We do not know what problem may be caused by having this error in the code. We hope the patch fixes that unknown problem. Even if it causes no problems, showing an exception at each closing of a tab is probably not something we want to expose users to in the stable TB24.

Comment 14

[Mark Banner (:standard8)]

Comment on attachment 813601 [details] [diff] [review]
882901.patch v2

[Triage Comment]
Needs landing on aurora/beta first, a=me for that.

Comment 15

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-aurora/rev/76a6d2d19d9d
https://hg.mozilla.org/releases/comm-beta/rev/61dd593ad635

Comment 16

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-esr24/rev/498bd3bd6d9b