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

RESOLVED FIXED in Thunderbird 27.0

Status

Thunderbird
Toolbars and Tabs
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ssitter, Assigned: aceman)

Tracking

24 Branch
Thunderbird 27.0

Thunderbird Tracking Flags

(thunderbird25 fixed, thunderbird26 fixed, thunderbird_esr2425+ fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

5.40 KB, patch
aceman
: review+
Details | Diff | Splinter Review
(Reporter)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
Created attachment 806860 [details] [diff] [review]
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).
Assignee: nobody → acelists
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #806860 - Flags: feedback?(syshagarwal)
Attachment #806860 - Flags: feedback?(ssitter)
(Assignee)

Comment 2

5 years ago
CCing guys involved in bug 868969 to check if the theory is correct.
Blocks: 868969
OS: Windows 7 → All
Hardware: x86_64 → All
Comment on attachment 806860 [details] [diff] [review]
882901.patch

Awesome, yes I can confirm the error is gone now :)
Attachment #806860 - Flags: feedback?(syshagarwal) → feedback+
(Assignee)

Comment 4

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

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

Thanks!
Attachment #806860 - Flags: review?(mconley)

Comment 5

5 years ago
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
(Assignee)

Comment 6

5 years ago
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

5 years ago
In such cases it's often useful to fix it, but also submit an additional diff with whitespace changes ignored for review.
Comment on attachment 806860 [details] [diff] [review]
882901.patch

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

Looks good! Thanks aceman.
Attachment #806860 - Flags: review?(mconley) → review+
(Assignee)

Comment 9

5 years ago
mconley, should I now do the reindenting patch?
Flags: needinfo?(mconley)
Yes - I should have been more clear: my r+ is conditional upon the fixes that Magnus asked for.
Flags: needinfo?(mconley)
(Assignee)

Comment 11

5 years ago
Created attachment 813601 [details] [diff] [review]
882901.patch v2

OK, thanks.
Attachment #806860 - Attachment is obsolete: true
Attachment #806860 - Flags: feedback?(ssitter)
Attachment #813601 - Flags: review+
(Assignee)

Updated

5 years ago
status-thunderbird24: --- → affected
tracking-thunderbird24: --- → ?
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/05ea0effa0a3
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 27.0
(Assignee)

Comment 13

5 years ago
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.
Attachment #813601 - Flags: approval-comm-esr24?
Comment on attachment 813601 [details] [diff] [review]
882901.patch v2

[Triage Comment]
Needs landing on aurora/beta first, a=me for that.
Attachment #813601 - Flags: approval-comm-beta+
Attachment #813601 - Flags: approval-comm-aurora+
status-thunderbird24: affected → ---
tracking-thunderbird24: ? → ---
tracking-thunderbird_esr24: --- → -
Attachment #813601 - Flags: approval-comm-esr24? → approval-comm-esr24+
tracking-thunderbird_esr24: - → 25+
You need to log in before you can comment on or make changes to this bug.