Closed
Bug 1432310
Opened 6 years ago
Closed 6 years ago
About dialog manual update URL is incorrect for beta
Categories
(Firefox :: General, enhancement, P1)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 60
Tracking | Status | |
---|---|---|
firefox60 | --- | fixed |
People
(Reporter: molly, Assigned: past)
References
Details
Attachments
(1 file)
The URL that we show in the about dialog when we fall back to offering a manual update is incorrect for beta channel builds. Since the URL comes from a pref (app.update.url.manual) that's set in firefox-branding.js, the same value is used for both release and beta. It's correct for release, but clicking on that link in a beta takes you to the release download page. For beta, the URL should have "/beta" appended to it.
Comment 1•6 years ago
|
||
That's controlled by Firefox prefs and the Firefox about UI so moving to Firefox -> General
Component: Application Update → General
Product: Toolkit → Firefox
Version: 59 Branch → unspecified
Assignee | ||
Comment 2•6 years ago
|
||
This is tricky to do. We deliberately make beta almost indistinguishable from release (name, icon, configuration settings), which is why we only have build-time flags like RELEASE_OR_BETA and EARLY_BETA_OR_EARLIER. Mike, is there any way to tweak browser/branding/official/pref/firefox-branding.js to use a different value for app.update.url.manual? Isn't this something we could do more easily on the server by parsing the UA string?
Flags: needinfo?(mh+mozilla)
Comment 3•6 years ago
|
||
Why not base that on app.update.channel? That's one of the few legitimate uses of that pref.
Flags: needinfo?(mh+mozilla)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 5•6 years ago
|
||
Thank you, that makes perfect sense.
Assignee: nobody → past
Status: NEW → ASSIGNED
Priority: -- → P1
Comment 6•6 years ago
|
||
mozreview-review |
Comment on attachment 8946501 [details] Bug 1432310 - Fix manual update URL for beta. https://reviewboard.mozilla.org/r/216508/#review222266 ::: browser/branding/official/pref/firefox-branding.js:14 (Diff revision 1) > pref("app.update.interval", 43200); // 12 hours > // Give the user x seconds to react before showing the big UI. default=192 hours > pref("app.update.promptWaitTime", 691200); > // URL user can browse to manually if for some reason all update installation > // attempts fail. > +#ifdef MOZ_UPDATE_CHANNEL == beta you want #if, not #ifdef. ::: browser/branding/official/pref/firefox-branding.js:21 (Diff revision 1) > +#else > pref("app.update.url.manual", "https://www.mozilla.org/firefox/"); > +#endif > // A default value for the "More information about this update" link > // supplied in the "An update is available" page of the update wizard. > pref("app.update.url.details", "https://www.mozilla.org/%LOCALE%/firefox/notes"); How about this one?
Attachment #8946501 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 7•6 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #6) > ::: browser/branding/official/pref/firefox-branding.js:21 > (Diff revision 1) > > +#else > > pref("app.update.url.manual", "https://www.mozilla.org/firefox/"); > > +#endif > > // A default value for the "More information about this update" link > > // supplied in the "An update is available" page of the update wizard. > > pref("app.update.url.details", "https://www.mozilla.org/%LOCALE%/firefox/notes"); > > How about this one? Not sure, Matt?
Flags: needinfo?(mhowell)
Reporter | ||
Comment 8•6 years ago
|
||
(In reply to Panos Astithas [:past] (please ni?) from comment #7) > (In reply to Mike Hommey [:glandium] from comment #6) > > ::: browser/branding/official/pref/firefox-branding.js:21 > > (Diff revision 1) > > > +#else > > > pref("app.update.url.manual", "https://www.mozilla.org/firefox/"); > > > +#endif > > > // A default value for the "More information about this update" link > > > // supplied in the "An update is available" page of the update wizard. > > > pref("app.update.url.details", "https://www.mozilla.org/%LOCALE%/firefox/notes"); > > > > How about this one? > > Not sure, Matt? Looks like that should also be changed, to "https://www.mozilla.org/%LOCALE%/firefox/beta/notes". That matches what we have on Nightly, and that URL does indeed redirect to the current beta release notes.
Flags: needinfo?(mhowell)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 10•6 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #6) > you want #if, not #ifdef. I looked through the repo and found another instance: https://searchfox.org/mozilla-central/rev/c56f656febb1aa58c72b218699e24a76425bd284/browser/app/profile/firefox.js#1736 Shall I fix it here or in another bug?
Comment 11•6 years ago
|
||
Please file a separate bug, and make it block bug 1419312.
Comment 12•6 years ago
|
||
mozreview-review |
Comment on attachment 8946501 [details] Bug 1432310 - Fix manual update URL for beta. https://reviewboard.mozilla.org/r/216508/#review222626 ::: browser/branding/official/pref/firefox-branding.js:18 (Diff revision 2) > -// attempts fail. > +// all update installation attempts fail. > +// app.update.url.details: a default value for the "More information about this > +// update" link supplied in the "An update is available" page of the update > +// wizard. > +#if MOZ_UPDATE_CHANNEL == beta > +pref("app.update.url.manual", "https://www.mozilla.org/firefox/beta"); Random thought: shouldn't those manual links also use %LOCALE% instead of relying on www.mozilla.org doing a possibly wrong redirection?
Attachment #8946501 -
Flags: review?(mh+mozilla) → review+
Comment 13•6 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #11) > Please file a separate bug, and make it block bug 1419312. Also, could you file a Core::Build config bug to make the preprocessor reject those constructs in the first place?
Assignee | ||
Comment 14•6 years ago
|
||
> Random thought: shouldn't those manual links also use %LOCALE% instead of
> relying on www.mozilla.org doing a possibly wrong redirection?
It's a valid suggestion, but I don't know if our server is properly set up to deal with every locale, so I'd rather leave it as is until I know more. I'll see if I can find someone who knows about this.
Comment 15•6 years ago
|
||
Pushed by pastithas@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/924b8c32b9a8 Fix manual update URL for beta. r=glandium
Assignee | ||
Comment 16•6 years ago
|
||
> Also, could you file a Core::Build config bug to make the preprocessor > reject those constructs in the first place? Filed bug 1434765.
Comment 17•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/924b8c32b9a8
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Comment 18•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=749b1b31bb0422aa8b4a78b350cdee93dd42ed11
Comment 19•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ba2998cb6f730206ce410cefccad940bde88bcb
You need to log in
before you can comment on or make changes to this bug.
Description
•