Closed Bug 1685222 Opened 5 months ago Closed 5 months ago

Uncaught SyntaxError: redeclaration of const THUNDERBIRD_THEME_PREVIEWS (duplicate observer created each time 'About Thunderbird' et alii get shown)

Categories

(Thunderbird :: Toolbars and Tabs, defect)

Thunderbird 85
defect

Tracking

(thunderbird_esr78 wontfix)

RESOLVED FIXED
86 Branch
Tracking Status
thunderbird_esr78 --- wontfix

People

(Reporter: worcester12345, Assigned: darktrojan)

References

(Regression)

Details

(Keywords: perf, regression)

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:85.0) Gecko/20100101 Firefox/85.0

Steps to reproduce:

Go to "Error Console"

Actual results:

Found this:
Uncaught SyntaxError: redeclaration of const THUNDERBIRD_THEME_PREVIEWS
<anonymous> chrome://messenger/content/aboutAddonsExtra.js:1
observe chrome://messenger/content/specialTabs.js:1719
aboutAddonsExtra.js:1:1
<anonymous> chrome://messenger/content/aboutAddonsExtra.js:1
observe chrome://messenger/content/specialTabs.js:1719

Expected results:

Not found this: ^^^

Component: Untriaged → Toolbars and Tabs

I don't see this error on Daily nor latest beta.

THUNDERBIRD_THEME_PREVIEWS is defined here: https://searchfox.org/comm-central/search?q=THUNDERBIRD_THEME_PREVIEWS&case=false&regexp=false&path=

Geoff, is this a problem with the map? You are much more better than I in JS, could you look at this?

Flags: needinfo?(geoff)

From code analysis, here's reliable STR, tested on TB 78.6.0 (64-bit) and Daily 86.0a1 (2021-01-06) (64-bit).

STR:

1.) Start TB with devtools, clear error console, ensure that about:addons-Tab is closed.
2.) Help > About Thunderbird [sic], open and close the about dialog. Repeat this step N times (say N = 5) [sic, for better analysis].
3.) Open add-ons tab: Tools > Add-ons
4.) Check error console, and how many occurrences of the same error (see below).

Actual result:

For N=5 in step 2, you'll find 5 instances of this error:

Uncaught SyntaxError: redeclaration of const THUNDERBIRD_THEME_PREVIEWS
    <anonymous> chrome://messenger/content/aboutAddonsExtra.js:1
    observe chrome://messenger/content/specialTabs.js:1717
aboutAddonsExtra.js:1:1
SyntaxError: redeclaration of const THUNDERBIRD_THEME_PREVIEWS aboutAddonsExtra.js:1:1

Code/error analysis ("what's going on here?"):

So to fix this bug, we may want to disentangle this and if we cannot eliminate the observer for about:addons, at least ensure that we don't create more than one instance of it. Geoff will know best how to fix this.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: perf
Summary: Uncaught SyntaxError: redeclaration of const THUNDERBIRD_THEME_PREVIEWS → Uncaught SyntaxError: redeclaration of const THUNDERBIRD_THEME_PREVIEWS (duplicate observer created each time 'About Thunderbird' et alii get shown)

Guess who the idiot was that put that there? :-)

Thanks for figuring this out Thomas. I'll move the observer somewhere else where it can exist only once.

Assignee: nobody → geoff
Status: NEW → ASSIGNED
Flags: needinfo?(geoff)
Regressed by: 1565180

(In reply to Thomas D. (:thomas8) from comment #2)

From code analysis,
...

  • Each time aboutAddonsExtra.js gets loaded by the duplicate observers, it will start with declaring the constant, and multiple declarations fail - good for us to uncover this nasty thing!
    ...
    So to fix this bug, we may want to disentangle this ... Geoff will know best how to fix this.

So, is there a way for me to clear this bug from "error console", and to test that it is now working better?
Thanks.

(In reply to Worcester12345 from comment #5)

So, is there a way for me to clear this bug from "error console", and to test that it is now working better?
Thanks.

Hey Worcester, if you're a little bit patient, we'll fix this for you and you can test this in Daily.
If you're impatient, you can try the patch in attachment 9195797 [details].
If you're not building, you can unzip omni.ja of your Daily into the folder where it resides, and tweak the two files manually as the patch does (specialTabs.js, MailGlue.jsm), then restart Daily to test.
Hth.

I'll wait. I thought it was already fixed and waiting to test. Just say when (and how).

(In reply to Worcester12345 from comment #7)

I'll wait. I thought it was already fixed and waiting to test. Just say when (and how).

Thank you! For most bugs, it'll only be fixed when the bug is marked FIXED.
The patch for this bug is being worked on in Phabricator, see attachment 9195797 [details].

Attachment #9195797 - Attachment description: Bug 1685222 - Move aboutAddonsExtra observer to MailGlue.jsm, where only one copy of it will be registered. r?ThomasD → Bug 1685222 - Fix how aboutAddonsExtra works to account for upstream changes. r?ThomasD
Target Milestone: --- → 86 Branch

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/46a5ae8ca59c
Fix how aboutAddonsExtra works to account for upstream changes. r=ThomasD

Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED

Of course there's a situation where this doesn't work – it's stupidly complicated for something so simple. And of course I forgot to do a Try run before I landed it.

Waiting for the load event like I did doesn't work in the case where the AOM is opened to the details view of an extension, because browserBundle isn't replaced before it is used. I'm going back to what I first proposed (sort of) because it removes a bunch of unnecessary complications. After a Try run, of course.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #9195797 - Attachment is obsolete: true

Geoff, pls set checkin-needed if a try-run with your latest patch looks good.

Flags: needinfo?(geoff)
Flags: needinfo?(geoff)

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/55562799bce8
Move aboutAddonsExtra observer to MailGlue.jsm, where only one copy of it will be registered. r=ThomasD

Status: REOPENED → RESOLVED
Closed: 5 months ago5 months ago
Resolution: --- → FIXED
Regressions: 1688428
Keywords: regression
You need to log in before you can comment on or make changes to this bug.