1547830 - Ensure updated WNP is shown in the Trailhead dot release (aka 67.0.5)

Status: VERIFIED FIXED

(Firefox :: Messaging System, task, P1)

Description

[Thomas Elin [:relaas]]

Hello Folks

Filing a bug to ensure our ability to show the updated 'Trailhead WNP' to users after updating to the Fx67 Trailhead dot release (aka 67.0.5)*

I'm not technical enough to articulate our current limitations but I've been told by Releng and confirmed with Tim Spurway that some level of in product work will be required to show the WNP after updating to a dot Release

It's important to note that Trailhead is targeting specific languages and locales; EN-US, FR, DE. This means that we'll need to architect this change to only show the updated WNP to that audience.

• Actual dot release version number TBD

Comment 1

[Ed Lee :Mardak]

The code shared with us was https://searchfox.org/mozilla-central/rev/b756e6d00728dda4121f8278a744381d8643317a/browser/components/BrowserContentHandler.jsm#531

Especially with the locale check, I wonder if it makes sense to handle opening a tab directly from asrouter ? although then we need to be careful about not triggering multiple welcome pages at slightly different times…

Comment 2

[Nick Thomas [:nthomas] (UTC+12)]

We might be able to solve this by landing a change like https://gist.github.com/nthomas-mozilla/760bc117b235ae08358f87f1ceec7604 for just 67.0.5. Anyone updating from 67.0/67.0.x to 67.0.5 would use the OVERRIDE_NEW_BUILD_ID case, and we'd treat it like it was an update to 67 for the first time. This means Balrog can tell Firefox what to do with a WNP page. The changes to whats_new_page.yml would mean the release automation only sets up WNP for the three locales requested, using the usual location.

Comment 3

[Ed Lee :Mardak]

[Tracking Requested - why for this release]: Special behavior for trailhead to show whats new page to show for existing users upgrading to 67.0.5

Comment 4

[Ed Lee :Mardak]

I believe the plan is to land this code specially and directly to 67.0.5 so we don't want this in 67 beta or 67.0rcs

Comment 5

[Ed Lee :Mardak]

Gijs, do you know who can help us figure out how to test this whats-new upgrade path logic? nthomas has a gist of a potential change in comment 2, but we're not sure if this is the right approach and what to look out for (as the logic seems to have a lot of corner cases).

The plan is we would land code directly in to 67.0.5, so we don't need to worry about triggering the new logic in 67.0, but we also don't want to unnecessarily re-show if there's a 67.0.6+.

I believe mozilla-release opens up for 67.0.5 in about a week on the ~21st?

Comment 6

[Ed Lee :Mardak]

(In reply to Nick Thomas [:nthomas] (UTC+12) from comment #2)

The changes to whats_new_page.yml would mean the release automation only sets up WNP for the three locales requested, using the usual location.

I don't think this will quite work as anyone upgrading from 67- directly to 67.0.5 should still see their usual WNP or at least the one for 67.0. Does this mean on the server side, it'll need to have different content for 67.0.5 en-US/de/fr vs say zh-TW?

66.0 -> 67.0   en-US = whats new 67.0
66.0 -> 67.0   zh-TW = whats new 67.0

66.0 -> 67.0.5 en-US = whats new 67.0.5
66.0 -> 67.0.5 zh-TW = whats new 67.0.5
67.0 -> 67.0.5 en-US = whats new 67.0.5
67.0 -> 67.0.5 zh-TW = no what's new

# if there's a 67.0.6…
66.0   -> 67.0.6 en-US = whats new 67.0.6
66.0   -> 67.0.6 zh-TW = whats new 67.0.6
67.0   -> 67.0.6 en-US = whats new 67.0.6
67.0   -> 67.0.6 zh-TW = no what's new
67.0.5 -> 67.0.6 en-US = no what's new
67.0.5 -> 67.0.6 zh-TW = no what's new

Comment 7

[Nick Thomas [:nthomas] (UTC+12)]

We can tweak versions on line 54 of the gist, I forgotten about that when I wrote it so it might be adding confusion now. I was expecting

66.0 -> 67.0   en-US = whats new 67.0
66.0 -> 67.0   zh-TW = whats new 67.0

66.0 -> 67.0.5 en-US = whats new 67.0.5
66.0 -> 67.0.5 zh-TW = no what's new
67.0 -> 67.0.5 en-US = whats new 67.0.5
67.0 -> 67.0.5 zh-TW = no what's new

although it's probably possible that we can do 'whats new 67.0' for the zh-TW case, if that's important. Depends what bedrock does for hosting the content; currently assuming the normal location will be used in the gist.

For 67.0.5 -> 67.0.6 I think we have a choice to re-show the WNP, if we want. It gets more complicated at 67.0.7 because we don't know if a user has seen it 0, 1, or 2 times already.

Comment 8

[:Gijs (he/him)]

(In reply to Ed Lee :Mardak from comment #5)

Gijs, do you know who can help us figure out how to test this whats-new upgrade path logic?

Not really. My first guess was nthomas. :-)

nthomas has a gist of a potential change in comment 2, but we're not sure if this is the right approach and what to look out for (as the logic seems to have a lot of corner cases).

It does have a lot of corner cases. It works for the basic idea we have today: updates to a new major version.

Have we already discounted the possibility of using 67.1 instead of 67.0.5 (presumably for other reasons)? AIUI that would avoid some of the problems here (ie no need to modify client code, dot releases after that still work the same way).

The other thing I recall is that we had something similar for 38.0.5 . Looks like :agibson and :nthomas might know more about how that worked at the time? I don't recall any specific client-side changes we made, but perhaps I'm just not remembering those bits?

The plan is we would land code directly in to 67.0.5, so we don't need to worry about triggering the new logic in 67.0, but we also don't want to unnecessarily re-show if there's a 67.0.6+.

Right, I think this is the tricky bit. With the changes in the gist, we will reshow things. If we back those changes out after 67.0.5 final is cut, then users who update 67->67.0.6 would not get them.

I don't see a good way around this, short of adding even more special-case code into that switch/case statement...

Comment 9

[Eric Renaud [:erenaud]]

Marketing is certainly okay with showing the content for 67.0.5 in subsequent dot versions. Ideally users updating from 67.0 (who would have seen the 67.0 content) directly to 67.0.6 (and later dot releases) would also see the content launched for 67.0.5.

Comment 10

[Ed Lee :Mardak]

So.. um. I tried to get the what's new page to open via BrowserContentHandler.jsm handling an upgrade, and I couldn't get it to open in 66.0.5 from 65.0. It's because there's no startup.homepage_override_url set, and it seems to be done that way since 6 years ago from bug 907404.

https://searchfox.org/mozilla-release/source/browser/branding/official/pref/firefox-branding.js#5

pref("startup.homepage_override_url", "");

I'm pretty sure this code is running as both browser.startup.homepage_override.{buildID,mstone} are getting updated from needHomepageOverride

I do see a what's-new-like page in that firefox-branding.js file:

pref("app.releaseNotesURL", "https://www.mozilla.org/%LOCALE%/firefox/%VERSION%/releasenotes/?utm_source=firefox-browser&utm_medium=firefox-browser&utm_campaign=whatsnew");

https://searchfox.org/mozilla-central/search?q=releaseNotesURL

But that looks like the release notes are only manually opened when the user clicks a "What's new" link from the "About Firefox" dialog or from about:preferences Updates section.

It seems like the straightforward (?) fix is to have some hardcoded url instead of reading out of homepage_override_url when we have the desired locales when triggering this update path on the mozilla-release code running in 67.0.5+.

nthomas, do you know what url we should be using?

Comment 11

[Ed Lee :Mardak]

I found bug 1077643 which basically does what we want and what I described at the end of comment 10:

https://hg.mozilla.org/releases/mozilla-release/rev/1d7405a097b1

Although, now I'm not sure how we currently have been showing the automatic what's new page. It seems like there is some update metadata that triggers it as per bug 538331…

Comment 12

[Nick Thomas [:nthomas] (UTC+12)]

When Firefox requests an update it may get some extra information back eg:
actions="showURL" openURL="https://www.mozilla.org/de/firefox/66.0.5/whatsnew/?oldversion=%OLD_VERSION%"

Then getPostUpdateOverridePage() replaces the empty value for startup.homepage_override_url with the value for openURL.

The update server can include/exclude the extra data based on version and locale conditions, so we could do this using the server and retain remote control. Or as you say bake it into the product for the 3 locales of interest.

Comment 13

[Nick Thomas [:nthomas] (UTC+12)]

Double check with someone like pmac for the actual url we're going to use.

Comment 14

[Tom Prince [:tomprince]]

I'm not at all familiar with the state of the in-product code to dispaly whats-new-pages. I do know that we now have code in the update server, that only serves the actions=showURL and openURL=... for updates coming from from previous milestones. I suspect the in-tree code to prevent displaying those predates the support in the update server. If that is the case, perhaps we don't need the in-tree limitation on when that is displayed, and leave that logic to the update server.

Comment 15

[Ed Lee :Mardak]

Ah thanks, that makes more sense. So there's 2 server aspects that might need changing depending on the client changes.

1. It seems like at minimum we'll want to treat OVERRIDE_NEW_BUILD_ID just like OVERRIDE_NEW_MSTONE as in comment 2's gist to potentially show an override url. AND update server needs to serve showURL/openURL
   1a) If the aus update information for all upgrades to 67.0.5 has a openURL="…/67.0.5/whatsnew/…" then mozilla.org would need to conditionally show "67.0.5" content for en/de/fr and "67.0" content for other locales.
   1b) Alternatively, update information could instead set openURL="…/67.0/whatsnew/…" for locales that shouldn't see 67.0.5 content, and other than someone looking at the url potentially being confused, it looks like the page content doesn't directly mention the version number. Although this might be confusing for metrics based on the version in url path not using the actual version.
   1c) Don't set openURL for upgrades to 67.0.5 from other locales even from a previous milestone, so the only people that should be seeing "…/67.0.5/whatsnew/…" would be en/de/fr users upgrading to 67.0.5.

2. If we don't want to change the update server logic right now, we can hardcode a url of https://www.mozilla.org/%LOCALE%/firefox/%VERSION%/whatsnew/?oldversion=%OLD_VERSION% instead of reading one from startup.homepage_override_url
   2a) Similar to 1a with mozilla.org needing to show the appropriate "67.0" content
   2b and 2c) like 1b/1c above except moving the update server locale checking logic into firefox. Shouldn't need special mozilla.org changes

Comment 16

[Tom Prince [:tomprince]]

I think something like 1c from comment 15 is the best option. The logic in the update server is data driven (whats_new_page.yml gets fed into the metadata for the release. Bug 1551749 is tracking the changes needed there to get the update server to serve the desired pages.

Comment 17

[Ed Lee :Mardak]

nthomas, it looks like your gist is what we want except also allowing en-CA and en-GB (unless the behavior is to just fall back to en-US and not worry about other en-* ??) Shall we assign this bug to you?

Probably a separate bug, it seems like the BrowserContentHandler.jsm changes could also go into mozilla-central so future releases have the ability for the update server to show a whats new page for build_id updates.

Comment 18

[Ed Lee :Mardak]

Attachment: Bug 1547830 - Ensure updated WNP is shown in the Trailhead dot release (aka 67.0.5)

Comment 19

[Nick Thomas [:nthomas] (UTC+12)]

I'm not best placed to own the BrowserContentHandler.jsm changes, but agree with Tom's suggestion of moving the browser/config/whats_new_page.yml changes out to bug 1551749.

Comment 20

[Ed Lee :Mardak]

Just making sure we only land this to mozilla-release for 67.0.x

Comment 21

[Ed Lee :Mardak]

Comment on attachment 9065935 [details]
Bug 1547830 - Ensure updated WNP is shown in the Trailhead dot release (aka 67.0.5)

Beta/Release Uplift Approval Request

• User impact if declined: No special trailhead what's new page on upgrade to 67.0.x
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: 1) create a new profile with 67.0

2. simulate update server data by setting startup.homepage_override_url to https://www.mozilla.org/firefox/%VERSION%/whatsnew/?oldversion=%OLD_VERSION%
3. quit and start 67.0.1 build with that profile
4. see tab loaded https://www.mozilla.org/en-US/firefox/67.0.1/whatsnew/?oldversion=67.0

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Tricky to test code path that needs update server changes from bug 1551749 but code changes are simple
• String changes made/needed: none

Comment 22

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9065935 [details]
Bug 1547830 - Ensure updated WNP is shown in the Trailhead dot release (aka 67.0.5)

Approved for 67.0.5 thanks.

Comment 23

[Ed Lee :Mardak]

https://hg.mozilla.org/releases/mozilla-release/rev/ad5f0d47928f96a6bb998da9eb52284f63ac036e

Comment 24

[Chris AtLee [:catlee]]

Perhaps we should wait to land that patch until we're closer to the trailhead dot release? In case we need to do a 67.0.x dot release?

Comment 25

[Axel Hecht [:Pike]]

With the branching strategy by Ritu, we're landing trailhead on default, and create a relbranch if we need a dot release before.

Comment 26

[Chris AtLee [:catlee]]

Ah, perfect! Carry on then :)

Comment 27

[Nick Thomas [:nthomas] (UTC+12)]

I've done a test 67.0.5 release to verify this fix in the full update flow. It's working as expected, but there is likely a bug in the profile downgrades protection.

Testing setup for mac. I started with en-US 67.0 in /Applications/Firefox.app/:

• change the update channel by adjusting Firefox.app/Contents/Resources/defaults/pref/channel-prefs.js to use pref("app.update.channel", "release-localtest");
• copy in an updater executable which accepts the dep-signed update from the staging release

  wget https://queue.taskcluster.net/v1/task/dpRosRixRXaG_kD62hiU-w/runs/1/artifacts/public/build/target.updater-dep.tests.tar.gz
  tar xf target.updater-dep.tests.tar.gz
  cp updater-dep/updater-dep /Applications/Firefox.app/Contents/MacOS/updater.app/Contents/MacOS/org.mozilla.updater
  

• in a terminal export MOZ_ALLOW_DOWNGRADE=1; open /Applications/Firefox.app
  • I think there's a profile downgrade bug that will need fixing before any 67.0.x, and need the export to avoid that. See Bug 1554029.
• created a new profile to avoid any cruft
• set app.update.auto pref to false using about:config. Also useful to set app.update.log to true to provide logs in the Browser Console.
• we need to modify app.update.url pref to use the staging update server, but the code looks at the default branch so do
  • open the Developer Tools, use the ... button to go to Settings. Check "Enable browser chrome and add-on debugging toolboxes"
  • open the Scratchpad (menu Web Developer > Scratchpad), switch to the Browser context using the Environment menu
  • paste in Services.prefs.getDefaultBranch(null).setCharPref("app.update.url", "https://stage.balrog.nonprod.cloudops.mozgcp.net/update/6/%PRODUCT%/%VERSION%/%BUILD_ID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/%OS_VERSION%/%SYSTEM_CAPABILITIES%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/update.xml"); alert('update url modified');
  • use the Run button to execute the code
• Check for an update by opening Firefox > About Firefox. The Browser Console will have lots of logging, including the update query URL if you want to inspect the xml returned

With all that I can update to the staging 67.0.5, and it opens the WNP at the regular location - https://www.mozilla.org/en-US/firefox/67.0.5/whatsnew/?oldversion=67.0, verifying the change here.

Caveats

• the staging release only builds locales en-CA, en-US, he, it, ja; only en-CA and en-US overlap with the trailhead locales
• this should work on other platforms too, with appropriate platform-specific steps. Alternate urls for the updater package are win64, win32, linux64, linux32

Comment 28

[Nick Thomas [:nthomas] (UTC+12)]

Also verified working on win64.

Comment 29

[Ciprian Muresan [:cmuresan], Ecosystem QA]

Looks like we're blocked from verifying the WNP because of bug 1554029. Leaving the ni? active until the bug is fixed and we can verify it.

Comment 30

[Tom Prince [:tomprince]]

Comment on attachment 9065935 [details]
Bug 1547830 - Ensure updated WNP is shown in the Trailhead dot release (aka 67.0.5)

Beta/Release Uplift Approval Request

• User impact if declined: We will not be able to display the trailhead WNP to users on beta updating from a 68 beta.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• String changes made/needed:

Comment 31

[Ed Lee :Mardak]

tomprince, what are we trying to fix with a mozilla-beta uplift? Users switching from 68.0bx to 67.0.x ? Shouldn't switching from 68.0 to 67.0 mstone trigger OVERRIDE_NEW_MSTONE and not need this patch?

Or is it that we want to show a whats new page for 68 beta upgrade to a newer 68 beta after a certain date?

Comment 32

[Tom Prince [:tomprince]]

There is also a request (Bug 1549889) to show the WNP on beta. So, somebody that is updating from 68.0b6 (May 31) to 68.0b7 (Jun 4) should see the WNP. My understanding is that this patch is need for that, for the same reason that it is need for showing the WNP on updates 67.0 -> 67.0.1.

Comment 33

[Ciprian Muresan [:cmuresan], Ecosystem QA]

I have verified that the WNP is correctly shown as the first tab when updating from 67.0 to 67.0.1 using the steps from comment 21 on Windows 10, macOS 10.14, and Arch Linux 4.14.3.

Comment 34

[Julien Cristau [:jcristau]]

Comment on attachment 9065935 [details]
Bug 1547830 - Ensure updated WNP is shown in the Trailhead dot release (aka 67.0.5)

show wnp even without a version bump, in support of trailhead. approved for 68.0b7

Comment 35

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6fc87a979fcb

Comment 36

[Marius Coman [:mcoman], Ecosystem QA]

I have verified that the WNP is correctly shown as the first tab when updating from 68.0b6 to 68.0b7 using the steps from comment 21 on Windows 10 x64, macOS 10.14.5, and Arch Linux 4.14.3.

Comment 37

[Tom Prince [:tomprince]]

Would it make sense to enable this by default on non-nightly branches?

Comment 38

[:Gijs (he/him)]

(In reply to Tom Prince [:tomprince] from comment #37)

Would it make sense to enable this by default on non-nightly branches?

I don't think so; it'd mean that if the update defines a what's new page for any beta/release build, we will show it even for security dot-releases and/or bN to bN+1 updates -- unless we permanently alter all our server-side logic to never include a what's new page for such updates (assuming we can do that for any/all such updates that aren't major version updates).

Comment 39

[Tom Prince [:tomprince]]

(In reply to :Gijs (he/him) from comment #38)

(In reply to Tom Prince [:tomprince] from comment #37)

Would it make sense to enable this by default on non-nightly branches?

I don't think so; it'd mean that if the update defines a what's new page for any beta/release build, we will show it even for security dot-releases and/or bN to bN+1 updates -- unless we permanently alter all our server-side logic to never include a what's new page for such updates (assuming we can do that for any/all such updates that aren't major version updates).

That code lives in-tree (here) and is already configured so that doesn't show the WNP for dot-releases or bN->bN+1 updates. That is, the server-side logic is already configured to support this code running on releases.

Comment 40

[:Gijs (he/him)]

(In reply to Tom Prince [:tomprince] from comment #39)

(In reply to :Gijs (he/him) from comment #38)

(In reply to Tom Prince [:tomprince] from comment #37)

Would it make sense to enable this by default on non-nightly branches?

I don't think so; it'd mean that if the update defines a what's new page for any beta/release build, we will show it even for security dot-releases and/or bN to bN+1 updates -- unless we permanently alter all our server-side logic to never include a what's new page for such updates (assuming we can do that for any/all such updates that aren't major version updates).

That code live in-tree (here) and is already configured so that doesn't show the WNP for dot-releases or bN->bN+1 updates. That is, the server-side logic is already configured to support this code running on releases.

Can you help me understand how that works? The conditional pattern there is <major.0 and I'd expect any beta version for a major version to match (but not release-channel dot updates). That is presumably 68.0b4 < 68.0 ? So I don't understand why/how this would work for beta...

Comment 41

[Tom Prince [:tomprince]]

(In reply to :Gijs (he/him) from comment #40)

Can you help me understand how that works? The conditional pattern there is <major.0 and I'd expect any beta version for a major version to match (but not release-channel dot updates). That is presumably 68.0b4 < 68.0 ? So I don't understand why/how this would work for beta...

The version number that gets sent to the update server does not include the beta number, so the update server (which evaluates the rules) would only see 68.0 for any 68 beta. It is possible to differentiate beta's, but currently only by their buildID (e.g. here for the trailhead page on beta).

---

There is version and displayVersion as version numbers. Only the later includes the bN.

Comment 42

[:Gijs (he/him)]

OK, then yes I think we could make such a change permanently going forward, for beta/release (using AppConstants to check for !nightly), leaving responsibility for avoiding update/whatsnew pages for beta/release completely in the hands of the server-side update data serving. Can you file a follow-up bug? Thank you!