1906983 - What's New page not displayed correctly after update

Status: RESOLVED FIXED

(Thunderbird :: General, defect, P1)

Description

[Andrei Hajdukewycz [:sancus]]

This function is not triggering correctly after updating to Thunderbird 128: https://searchfox.org/comm-esr128/rev/38abeb35c357a3032d84535d7c26da4ea37c4605/mail/base/content/specialTabs.js#1022

Possibly because there is no import for openLinkExternally.

This will need to be fixed before we launch 128. The What's New page should pop up on the first startup after updating, and then the donation appeal should appear on the 2nd restart.

Comment 1

[Geoff Lankow (:darktrojan)]

The function isn't missing. Is it a Balrog problem? https://searchfox.org/comm-esr128/source/mail/branding/include/release-prefs.js#4

Comment 2

[Andrei Hajdukewycz [:sancus]]

(In reply to Geoff Lankow (:darktrojan) from comment #1)

The function isn't missing. Is it a Balrog problem? https://searchfox.org/comm-esr128/source/mail/branding/include/release-prefs.js#4

Not sure, Rob was the one who suggested it might be an import issue.

Comment 3

[Alessandro Castellani [:aleca] - PTO until Nov 16]

Wasn't that updated here? https://hg.mozilla.org/comm-central/rev/2405535297f5f50389d9f1678cd286f571caf046

Comment 4

[Alessandro Castellani [:aleca] - PTO until Nov 16]

Attachment: Bug 1906983 - Import the missing openLinkExternally() method. r=#thunderbird-reviewers

Comment 5

[Alessandro Castellani [:aleca] - PTO until Nov 16]

I uploaded a patch with the missing import, just in case that's the issue.
I can't test it right now because my local build is busted, trying to troubleshoot it.

Comment 6

[Rob Lemley [:rjl]]

I have high confidence this is the issue.

• https://searchfox.org/comm-esr128/rev/38abeb35c357a3032d84535d7c26da4ea37c4605/mail/base/content/messenger.js#174
  calls specialTabs.openSpecialTabsOnStartup()
• https://searchfox.org/comm-esr128/rev/38abeb35c357a3032d84535d7c26da4ea37c4605/mail/base/content/specialTabs.js#740
  calls this.showWhatsNewPage();
• https://searchfox.org/comm-esr128/rev/38abeb35c357a3032d84535d7c26da4ea37c4605/mail/base/content/specialTabs.js#1040-1046 is the fun...
  the override_url pref is the fallback default, usually empty. in getPostUpdateOverridePage it can be overridden by a "showURL" action in the XML returned by Balrog. That looks like this: (as of last night, abbreviated for clarity)

<?xml version="1.0"?><updates xmlns="http://www.mozilla.org/2005/app-update"><update xmlns="http://www.mozilla.org/2005/app-update" appVersion="115.12.2" buildID="20240621154414" channel="esr" detailsURL="https://live.thunderbird.net/thunderbird/releasenotes?locale=en-US&amp;version=115.12.2&amp;channel=esr" displayVersion="115.12.2" installDate="1720622824411" isCompleteUpdate="true" name="Thunderbird 115.12.2" previousAppVersion="115.12.0" promptWaitTime="86400" serviceURL="https://aus5.mozilla.org/update/6/Thunderbird/115.12.0/20240610193835/Linux_x86_64-gcc3/en-US/esr/Linux%206.9.8-zen1-1-zen%20(GTK%203.24.42%2Clibpulse%2017.0.0)/ISET:SSE4_2,MEM:128585/default/default/update.xml" type="minor" statusText="The Update was successfully installed"><patch size="62676836" type="complete" URL="https://download.mozilla.org/?product=thunderbird-115.12.2-complete&amp;os=linux64&amp;lang=en-US" finalURL="https://download-installer.cdn.mozilla.net/pub/thunderbird/releases/115.12.2/update/linux-x86_64/en-US/thunderbird-115.12.2.complete.mar" selected="true" state="succeeded" hashFunction="sha512" hashValue="03eabd9b8b106f289dcd4439f2c4d7b289211d61f67af2fb80edbf1f282000e21c8fc47de8b13b168a9729fd21d6f009abb346db77e6618638d5157af70dd531" internalResult="0"/></update></updates>

Assuming there's no "silent" in the update XML and the enterprise policy is not blocking it, the openURL property has the what's new page URL which is passed to openLinkExternally. (Side note, the "detailsURL" field is where the release notes URL comes from that is populated into the about: dialog and a few other places.)

Testing this scenario is a bit of a pain outside of a release context. There is toolkit/mozapps/update/tests/browser/browser_showWhatsNewPageTest.js which could probably be adapted for Thunderbird.

Comment 7

[Martin Giger [:freaktechnik]]

I don't think the patch from aleca is the correct approach to resolve this.

When I bypass the MOZ_UPDATER appconstant check in a local build the page opens just fine from what I can tell - however I haven't managed to get a working ESR build. I think my adaptation of that test you referred to does actually manage to pull the value from the XML, because I don't see any other way of it getting the test value - even though it seems to be opened before the test actually manages to run.

If some JS is failing you should see that in the error console of the build at least, and I see no error logs here so far?
Do you have any more concrete reproduction instructions?

Comment 8

[Martin Giger [:freaktechnik]]

If I try to apply the XML you pasted, it points out that it's not valid because of isCompleteUpdate "true" instead of isCompleteUpdate="true".

EDIT: if I correct that the page gets correctly opened.

Comment 9

[Rob Lemley [:rjl]]

The call to um.getReadyUpdate() fails here: https://searchfox.org/comm-esr128/rev/38abeb35c357a3032d84535d7c26da4ea37c4605/mail/base/content/specialTabs.js#1037(In reply to Martin Giger [:freaktechnik] from comment #8)

If I try to apply the XML you pasted, it points out that it's not valid because of isCompleteUpdate "true" instead of isCompleteUpdate="true".

Gah! Copy/paste problem. The "=" is there. Fixed above.

Comment 10

[Alessandro Castellani [:aleca] - PTO until Nov 16]

(In reply to Martin Giger [:freaktechnik] from comment #8)

If I try to apply the XML you pasted, it points out that it's not valid because of isCompleteUpdate "true" instead of isCompleteUpdate="true".

EDIT: if I correct that the page gets correctly opened.

Good catch, thanks for confirming.
I also had big suspicion that my patch was kinda useless and not the cause of the problem due to no console errors.

Comment 11

[Rob Lemley [:rjl]]

what i've been doing is

• install 115 & setting up a mailbox
• set the update channel to esr-localtest-next.
• run the update and stop tb
• grab specialtabs.js out of updated/omni.ja, and stick a "debugger" line at the beginning of showWhatsNewPage()
• start tb with --jsdebugger --wait-for-jsdebugger
• then step through. i fail at the um.getReadyUpdate() line as it returns null.

Comment 12

[Andrei Hajdukewycz [:sancus]]

(In reply to Martin Giger [:freaktechnik] from comment #8)

If I try to apply the XML you pasted, it points out that it's not valid because of isCompleteUpdate "true" instead of isCompleteUpdate="true".

EDIT: if I correct that the page gets correctly opened.

As far as I can tell, the XML mistake was only here on Bugzilla. It's not working even with the correct XML on 115.

Comment 13

[Martin Giger [:freaktechnik]]

I think the fix is

diff --git a/mail/base/content/specialTabs.js b/mail/base/content/specialTabs.js
--- a/mail/base/content/specialTabs.js
+++ b/mail/base/content/specialTabs.js
@@ -1019,7 +1019,7 @@ var specialTabs = {
    *
    * @see {BrowserContentHandler.needHomepageOverride}
    */
-  async showWhatsNewPage() {
+  showWhatsNewPage() {
     const old_mstone = Services.prefs.getCharPref(
       "mailnews.start_page_override.mstone",
       ""
@@ -1034,7 +1034,7 @@ var specialTabs = {
       const um = Cc["@mozilla.org/updates/update-manager;1"].getService(
         Ci.nsIUpdateManager
       );
-      const update = await um.getReadyUpdate();
+      const update = um.updateInstalledAtStartup;
 
       if (update && Services.vc.compare(update.appVersion, old_mstone) > 0) {
         let overridePage = Services.urlFormatter.formatURLPref(

correcting a porting mistake from bug 1897367.

Comment 14

[Rob Lemley [:rjl]]

I can confirm this fix does work -- ran with patched omni.ja without using debugger. (both alex's and martin's patches applied)

Comment 15

[Martin Giger [:freaktechnik]]

Attachment: Bug 1906983 - Fix what's new page not showing up after update. r=#thunderbird-reviewers

Comment 16

[Martin Giger [:freaktechnik]]

(In reply to Rob Lemley [:rjl] from comment #14)

(both alex's and martin's patches applied)

I've been testing without alex's patch and that's worked fine, so I don't think we need that one.

Comment 17

[Andrei Hajdukewycz [:sancus]]

Comment on attachment 9412094 [details]
Bug 1906983 - Fix what's new page not showing up after update. r=#thunderbird-reviewers

[Approval Request Comment]
Regression caused by (bug #): Bug 1897367
User impact if declined: No what's new page
Testing completed (on c-c, etc.): c-c
Risk to taking this patch (and alternatives if risky): Little other than the What's New page still being broken.

Comment 18

[Corey Bryant]

Comment on attachment 9412094 [details]
Bug 1906983 - Fix what's new page not showing up after update. r=#thunderbird-reviewers

[Triage Comment]
Approved for esr128

Comment 19

[Martin Giger [:freaktechnik]]

I think beta might need a different patch, there were more changes on m-c for 129 in this area I'm currently looking into.

Comment 20

[Andrei Hajdukewycz [:sancus]]

(In reply to Martin Giger [:freaktechnik] from comment #19)

I think beta might need a different patch, there were more changes on m-c for 129 in this area I'm currently looking into.

If the patch doesn't work on trunk, we should just uplift this one, and check a normal fix into comm-central and let it ride the merges. Beta doesn't need a what's new page right now.

Comment 21

[Corey Bryant]

Comment on attachment 9412094 [details]
Bug 1906983 - Fix what's new page not showing up after update. r=#thunderbird-reviewers

Removed approval-comm-beta in favor of picking up the comm-central fix in the next merge.

Comment 22

[Martin Giger [:freaktechnik]]

Pushed to ESR: https://hg.mozilla.org/releases/comm-esr128/rev/5b8f11ce4188f2d46aa8021a390e543e92a23e0d

I'll provide a patch for comm-central that also includes a test to cover this code and it'll follow the latest changes in m-c, too.

Comment 23

[Martin Giger [:freaktechnik]]

Attachment: Bug 1906983 - Fix what's new page after update and add tests. r=#thunderbird-reviewers

This is the continuation of the changes needed for ESR including porting the browser_showWhatsNewPageTest.js
from mozilla-central. I've stripped all the support files to only include things used by this test except the
setup/teardown in the head file. Further I've ignored the platformVersion vs. appVersion things Firefox does,
since it seems we only care about appVersion.

Comment 24

[Martin Giger [:freaktechnik]]

Please check in just the second, newer patch that hasn't been uplifted to ESR.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/4c6d065ee010
Fix what's new page after update and add tests. r=mkmelin