Closed Bug 1563411 Opened 5 years ago Closed 4 years ago

Port bug 1555088 - Use activeUpdate and an app version increase to determine that an update was performed and remove the post update pref

Categories

(Thunderbird :: Upstream Synchronization, task)

task
Not set
normal

Tracking

(thunderbird_esr78+ fixed, thunderbird81 fixed)

RESOLVED FIXED
82 Branch
Tracking Status
thunderbird_esr78 + fixed
thunderbird81 --- fixed

People

(Reporter: robert.strong.bugs, Assigned: mkmelin)

References

Details

Attachments

(2 files, 2 obsolete files)

Spinoff of bug 1555088 for Thunderbird

The app.update.postupdate preference will be removed in bug 1555088 to remove a file read during startup.

The following patch in phab has the changes needed for Thunderbird
https://phabricator.services.mozilla.com/D36853

Since Thunderbird isn't using the update xml to specify the url to open or anything else you might consider just doing this on version change and not bother with the update at all.

Summary: Use activeUpdate and an app version increase to determine that an update was performed and remove the post update pref → Port bug 1555088 - Use activeUpdate and an app version increase to determine that an update was performed and remove the post update pref

Perhaps we can get Richard interested.

I looked at it and don't know where to put the changes and what changes we need. This is for someone that knows how this all works, sorry.

Now that bug 1555088 landed someone should look at this bug.

I noticed this during testing. We need to fix this now. Due to this bug we aren't showing the what'snew page to up-graders.

Assignee: nobody → mkmelin+mozilla
Component: General → Upstream Synchronization
Attached patch bug1563411_whatsnew_open.patch (obsolete) — Splinter Review

This is grabbing some Firefox code. It's in the wrong file nowadays, bug we can move that later after bug 1662665.

Not sure how to easily test this. Can the update.xml be faked somehow?

Attachment #9173568 - Flags: review?(geoff)
Status: NEW → ASSIGNED

:rjl, is the showURL property set in our update.xml files? And do you have some sample urls?

Flags: needinfo?(rob)

ISTR from last time I was looking at this stuff (a long time ago, admittedly) that our updates don't/can't have any of the properties this code uses. Not that that would stop it from working.

We don't have a showURL property in our updates.
I can make Balrog send it easily enough, but I have similar recollection as Geoff. Last time I tried to set something up I could not get it to work because the update code in Thunderbird was missing some functionality.
I can try to set something up on a test channel so we can see what it does.
As for the URL, it's pretty much whatever. I just stick it in a YAML file. See the one for Firefox at browser/config/whats_new_page.yml.

https://www.mozilla.org/%LOCALE%/{product}/{version}/whatsnew/?oldversion=%OLD_VERSION%
product and version are filled in by Taskcluster at release promotion time and %LOCALE% %OLD_VERSION% by Balrog.

Flags: needinfo?(rob)

I think we don't need to get the url from the update (the openURL property). We have a URL to use.
But I want to know if action="silent" is there for upgrades between point releases. If it's not, then with the patch a what's new would be shown for them too which is... perhaps a bit much ;)
If we want to have the what's new showing then action="showURL" should be used.

The whats-new-page setup handles all that. Well except for action="silent". Does that even exist? I don't think it's needed though.

We would add to the existing file in comm/mail/config/whats_new_page.yml something like:

- type: show-url
  url: <whatever url -- if its localized, then great can handle that too>
  conditions:
    blob-types: [wnp]
    release-types: [release]
    products: [thunderbird]
    update-channel: [release]
    versions: ["<{version.major_number}.0"]

The versions line sets up the condition you're looking for. Taskcluster will expand that to ["<78.0"] and then on Balrog the release blob will contain:

"updateLine": [
        {
            "for": {},
            "fields": {
                "detailsURL": "https://live.thunderbird.net/thunderbird/releasenotes?locale=%LOCALE%&version=78.2.2&channel=release",
                "type": "minor"
            }
        },
        {
            "for": {
                "locales": [<list of locales>],
                "versions": ["<78.0"]
            },
            "fields": {
                "actions": "showURL",
                "openURL": "<the whats new url>"
            }
        }
    ],

When updating from 68 the update line would be:

<update actions="showURL" appVersion="78.2.2" buildID="20200902163820" detailsURL="https://live.thunderbird.net/thunderbird/releasenotes?locale=en-US&amp;version=78.2.2&amp;channel=release" displayVersion="78.2.2" openURL="<the whats new url>" type="minor">

And when updating from easier 78:

<update appVersion="72.2.2" buildID="20200902163820" detailsURL="https://live.thunderbird.net/thunderbird/releasenotes?locale=en-US&amp;version=78.2.2&amp;channel=release" displayVersion="78.2.2" type="minor">
Comment on attachment 9173568 [details] [diff] [review]
bug1563411_whatsnew_open.patch

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

::: mail/branding/nightly/thunderbird-branding.js
@@ +5,5 @@
>  // Default start page
>  pref("mailnews.start_page.url", "https://live.thunderbird.net/%APP%/start?locale=%LOCALE%&version=%VERSION%&channel=%CHANNEL%&os=%OS%&buildid=%APPBUILDID%");
>  
>  // start page override to load after an update
> +pref("mailnews.start_page.override_url", "https://live.thunderbird.net/%APP%/whatsnew?locale=%LOCALE%&version=%VERSION%&channel=%CHANNEL%&os=%OS%&buildid=%APPBUILDID%&oldversion=%OLD_VERSION%");

I think we can set this to "", as that's the case with Firefox. By leaving it set getPostUpdateOverridePage() will return this value when there is no action set. (Line 1077 and 1093)

::: mail/branding/thunderbird/thunderbird-branding.js
@@ +1,5 @@
>  // Default start page
>  pref("mailnews.start_page.url", "https://live.thunderbird.net/%APP%/start?locale=%LOCALE%&version=%VERSION%&channel=%CHANNEL%&os=%OS%&buildid=%APPBUILDID%");
>  
>  // start page override to load after an update
> +pref("mailnews.start_page.override_url", "https://live.thunderbird.net/%APP%/whatsnew?locale=%LOCALE%&version=%VERSION%&channel=%CHANNEL%&os=%OS%&buildid=%APPBUILDID%&oldversion=%OLD_VERSION%");

Same here, set to "".
Attachment #9173883 - Flags: review?(mkmelin+mozilla)

Seems firefox does set it (their corresponding pref) on nightly - https://searchfox.org/comm-central/search?q=%22startup.homepage_override_url%22&path=

Attachment #9173883 - Flags: review?(mkmelin+mozilla) → review+
Attached patch bug1563411_whatsnew_open.patch (obsolete) — Splinter Review
Attachment #9173568 - Attachment is obsolete: true
Attachment #9173568 - Flags: review?(geoff)
Attachment #9173888 - Flags: review?(geoff)
Comment on attachment 9173888 [details] [diff] [review]
bug1563411_whatsnew_open.patch

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

Just the one question.

::: mail/branding/thunderbird/thunderbird-branding.js
@@ -12,5 @@
>  pref("app.update.promptWaitTime", 86400);
>  
>  pref("app.vendorURL", "https://www.thunderbird.net/%LOCALE%/");
> -
> -pref("browser.search.param.ms-pc", "MOZT");

Is this intentional?
Attachment #9173888 - Flags: review?(geoff) → review+

Comment on attachment 9173883 [details] [diff] [review]
bug1563411_whatsnew_config.patch

[Approval Request Comment]
The patches on this bug will bring our updater closer to Firefox's. The important part here is the "whats new page" feature which has been on Firefox for some time, but Thunderbird has not supported. As Magnus points out there is nothing shown on upgrade from 68->78 due to the toolkit changes that triggered this bug. From a donation perspective that's important.

Attachment #9173883 - Flags: approval-comm-esr78?
Attachment #9173883 - Flags: approval-comm-beta?

Comment on attachment 9173883 [details] [diff] [review]
bug1563411_whatsnew_config.patch

[Triage Comment]
Approved for beta.
Conditionally approved for esr. (assuming testing goes well - tagging Walt, but we will try to have several people test)

Flags: needinfo?(wls220spring)
Attachment #9173883 - Flags: approval-comm-esr78?
Attachment #9173883 - Flags: approval-comm-esr78+
Attachment #9173883 - Flags: approval-comm-beta?
Attachment #9173883 - Flags: approval-comm-beta+

Pushed by thunderbird@calypsoblue.org:
https://hg.mozilla.org/comm-central/rev/8828fc048e0d
Port bug 1555088 - Use activeUpdate and an app version increase to determine that an update was performed and remove the post update pref. r=rjl
https://hg.mozilla.org/comm-central/rev/ae581bdbf75b
Configure a whats-new-page for Thunderbird to display release notes on major updates. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch

This is what actually landed - without the search pref line removed since it is unrelated to this bug.

Attachment #9173888 - Attachment is obsolete: true
Pushed by thunderbird@calypsoblue.org:
https://hg.mozilla.org/comm-central/rev/3dd44dbfb486
Follow-up: Fix yamllint errors. rs=linting DONTBUILD

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

-pref("browser.search.param.ms-pc", "MOZT");

Is this intentional?

It was. That is some old junk no not used for anything anymore. Oh well, we can remove it some other time.

When testing, I noticed that this page opens in a web browser, while the updated privacy policy opens in a tab within Thunderbird. Can the whats new page be opened in a Thunderbird tab as well? It would be more consistent.

Flags: needinfo?(mkmelin+mozilla)

It's very intentional. From the internal web page people can't really explore the other Thunderbird pages, i.e. can't "get involved", and "can't" really donate. The privacy tab could perhaps also be opened in the browser, but then it's not as clear that it's for Thunderbird.

Flags: needinfo?(mkmelin+mozilla)
Regressions: 1664607

Just tested the update to 78.2.2 from 68.12.0 on Ubuntu 18.04 following the instructions provided in an email.

The 78.0 release notes opened in Firefox.

Flags: needinfo?(wls220spring)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: