[go: nahoru, domu]

Closed Bug 1906983 Opened 1 month ago Closed 1 month ago

What's New page not displayed correctly after update

Categories

(Thunderbird :: General, defect, P1)

Thunderbird 128

Tracking

(thunderbird_esr128? fixed, thunderbird129 affected, thunderbird130 fixed)

RESOLVED FIXED
130 Branch
Tracking Status
thunderbird_esr128 ? fixed
thunderbird129 --- affected
thunderbird130 --- fixed

People

(Reporter: sancus, Assigned: freaktechnik)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

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.

Severity: -- → S1
OS: Unspecified → All
Priority: -- → P1
Hardware: Unspecified → All
Assignee: nobody → alessandro
Status: NEW → ASSIGNED

(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.

Flags: needinfo?(rob)

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.

I have high confidence this is the issue.

<?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.

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?

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.

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.

Flags: needinfo?(rob)

(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.

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.

(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.

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.

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

(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.

Keywords: regression
Regressed by: 1897367
Attachment #9411920 - Attachment is obsolete: true
Assignee: alessandro → martin

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.

Attachment #9412094 - Flags: approval-comm-esr128?

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

Attachment #9412094 - Flags: approval-comm-esr128?
Attachment #9412094 - Flags: approval-comm-esr128+
Attachment #9412094 - Flags: approval-comm-beta+

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.

(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 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.

Attachment #9412094 - Flags: approval-comm-beta+

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.

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.

Blocks: tb128found

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

Target Milestone: --- → 130 Branch

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/4c6d065ee010
Fix what's new page after update and add tests. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: