1664607 - bug 1563411 breaks UI for builds with updater disabled

Status: RESOLVED FIXED

(Thunderbird :: Upstream Synchronization, defect)

Description

[Landry Breuil (:gaston)]

78.2.1 was fine, 78.2.2 has the account wizard broken at startup, or message pane empty, etc.. in the error console this message is shown:

Uncaught TypeError: Cc['@mozilla.org/updates/update-manager;1'] is undefined

https://hg.mozilla.org/releases/comm-esr78/rev/ec20676c465e72ce5a35db0f4df995f4a508a24c#l1.78 doesnt check if the update-manager is available ?

Comment 1

[Wolfgang Rosenauer [:wolfiR]]

In addition apparently the "Check for updates" button also appears even if built w/o updater:
ac_add_options --disable-updater
which is another regression.

Comment 2

[Wolfgang Rosenauer [:wolfiR]]

wait a sec and forget my last comment for now. Did some test mistake.

Comment 3

[Landry Breuil (:gaston)]

Uncaught TypeError: Cc['@mozilla.org/updates/update-manager;1'] is undefined
    showWhatsNewPage chrome://messenger/content/specialTabs.js:1051
    openSpecialTabsOnStartup chrome://messenger/content/specialTabs.js:799
    OnLoadMessenger chrome://messenger/content/msgMail3PaneWindow.js:617
    onload chrome://messenger/content/messenger.xhtml:1

is the complete error message.

From my understanding, other similar callers do call .activeUpdate after getting the potential updatemanager component, eg https://searchfox.org/comm-esr78/source/mail/components/preferences/general.js#1484 ? or that should be withing some #ifdef updater-is-enabled ?

Comment 4

[Landry Breuil (:gaston)]

missing check for AppConstants.MOZ_UPDATER maybe ?

Comment 5

[Landry Breuil (:gaston)]

looking at the showWhatsNewPage() method in .1 and .2, previously there was a try/catch to gracefully handle non-existent update-manager component, which isnt the case anymore. Dunno what's the best fix for all cases, but for now adding a local hack.

Comment 6

[Wayne Mery (:wsmwk)]

like bug 1664590 ?

Comment 7

[Frank K]

See also openSUSE bug at https://bugzilla.opensuse.org/show_bug.cgi?id=1176384#c0

Comment 8

[Landry Breuil (:gaston)]

(In reply to Wayne Mery (:wsmwk) from comment #6)

like bug 1664590 ?

cant say that the exact same bug but seems similar

Comment 10

[Rob Lemley [:rjl]]

Attachment: Bug 1664607 - Don't try to load what's new page when built with updater disabled. r=mkmelin

When Thunderbird is built with --disable-updater, as it done by most Linux
distributions, accessing the nsIUpdateManager service will throw an error
resulting in a broken UI. Check AppConstants.MOZ_UPDATER when using
nsIUpdateManger to prevent errors.

Comment 11

[Rob Lemley [:rjl]]

I did a local build with this patch and --disable-updater and verified that it works as intended. Try jobs at:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=b0c4a8521444fea8af25aee1f9619818a7e5b7a5
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=aa4b9d60192ae7f1ca9f39c0cdedb7fa6c19f28a

I also quickly checked to see what happens when updates are disabled via enterprise policies. In that case, activeUpdate should just return null so there should not be any problems.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/6379c3b860f7
Don't try to load what's new page when built with updater disabled. r=mkmelin DONTBUILD

Comment 14

[Rob Lemley [:rjl]]

Comment on attachment 9175384 [details]
Bug 1664607 - Don't try to load what's new page when built with updater disabled. r=mkmelin

[Approval Request Comment]
Regression caused by (bug #): 1563411
User impact if declined: This issue affects users that use various Linux distribution builds which are built with --disable-updater. The bug causes portions of the UI to not load resulting in an unusable interface. Users of "official" builds are not affected, and the fix would have no impact as it just wraps the what-new loading code in a conditional based on the value of MOZ_UPDATER which is set at build time automatically.
Testing completed (on c-c, etc.): I tested local a build of esr78 with this patch and --disable-updater to make sure it works as expected. When testing beta it would be nice if QA can test with an enterprise policy that disables updates. I don't expect any issues based on reading the code, but I may have missed something.
Risk to taking this patch (and alternatives if risky): I see very little risk in accepting this patch as the variable in question is always set to True on official builds and is not possible to change without a recompile.

Comment 15

[Wayne Mery (:wsmwk)]

Comment on attachment 9175384 [details]
Bug 1664607 - Don't try to load what's new page when built with updater disabled. r=mkmelin

[Triage Comment]
Approved for beta
Approved for esr78

[Triage Comment]

Comment 16

[Rob Lemley [:rjl]]

Thunderbird 81.0b4:
https://hg.mozilla.org/releases/comm-beta/rev/e272532e6010

Comment 17

[Rob Lemley [:rjl]]

I checked an official build with enterprise policies disabled and saw no UI issues.
OpenSUSE has applied this patch to their 78.2.2 build Bug and Changelog successfully.

Comment 18

[Rob Lemley [:rjl]]

Thunderbird 78.3.0:
https://hg.mozilla.org/releases/comm-esr78/rev/f085dbd311bc