See spec: https://mozilla.invisionapp.com/share/P4ACQT1E3#/screens/217167550 This functionality should be copied from the About dialog. As part of this work we should make a shared module so that the code doesn't get duplicated.

Hi Evan, can you pick this up? We might need to add a string for this ("Version %S") and I'd like to get this added sooner rather than later since it introduces a new string.

Hi Jared, Since the Project Photon is our team's top priority task, I'm working on Photon-related bugs. We might need to confirm this bug is included in the Photon scope before I work on that. And this is Cindy's call. Cindy, do you know is this included in the Photon scope? Thanks.

Hi Evan, Yes this one is within the scope. I've added the whiteboard tag to track it.

(In reply to (Away 3 May - 5 May) Jared Wein [:jaws] (please needinfo? me) from comment #0) > Created attachment 8858235 [details] > Screenshot of spec > > See spec: https://mozilla.invisionapp.com/share/P4ACQT1E3#/screens/217167550 > > This functionality should be copied from the About dialog. As part of this > work we should make a shared module so that the code doesn't get duplicated. Does this also replicate the update checking and "Apply update" button? IIRC there were some old limitations on having this stuff displayed in multiple places (e.g. having multiple about:prefs tabs open, or both this page and the About dialog). I can't recall exactly what the issue was or if it still apply... rstrong, you remember this?

The info in the about window is not entirely accurate since when that was implemented it was decided that it should state that the browser is up to date when there was an update check failure. The "up to date' message is determined by the update check not finding an update whether an error occurred during the check or not. There are issues with performing update checks simultaneously from the about window and in the toolkit app update UI simultaneously since the UI's wouldn't show the same state.

Also, when there are repeated consecutive failures we do inform the client. This handles the case where the client is unable to contact the update server for whatever reason without unduly alarming them by notifying them when it is a transient error.

this bug should also include an "update firefox" button if the current version is not the latest. tina will add specs here.

From my end of things I'm focused on making updates as silent and reliable as possible. It should be possible to duplicate the code from the Firefox about dialog to add this functionality for the few users that would get value out of this.

Comment 5 is a bit troubling. I don't think it's feasible to refactor aboutDialog-appUpdater.js in the fx55 cycle to allow multiple UIs update at the same time. If we could accept that as a post-55 follow-up we should not try to get this UI into fx55. Cindy, what do you think?

(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #9) > Comment 5 is a bit troubling. I don't think it's feasible to refactor > aboutDialog-appUpdater.js in the fx55 cycle to allow multiple UIs update at > the same time. If we could accept that as a post-55 follow-up we should not > try to get this UI into fx55. Cindy, what do you think? I think it's reasonable to push it to after 55.

Tina, For the release version, on the about dialog we will have a "What's New" link followed by the version number. Also if the Firefox is a distribution build, we will show the distribution and distribution ID on the second line. See attachment 8572733 [details] from bug 1139509. Do you want to show the "what's new" link? Do you want to show, and what's the proposed style of, distribution desc and ID here?

(Tentatively assign to myself. Might ask other to grab if occupied)

I found another screenshot on distribution label: https://www.soeren-hentzschel.at/wp-content/uploads/2012/07/firefox-funnelcake.png

Comment on attachment 8862392 [details] Bug 1356507 - Show version and updater in the preferences update pane. I used the existing XUL components to put the labels without the spec. Tina should check if they are alright. Jared, this patch is on top of bug 1330121 and it ... works. I wonder if you would like to duplicate tests to test the updater also, or rename the aboutDialog-appUpdater.js. I'll update the patch accordingly.

Comment on attachment 8862392 [details] Bug 1356507 - Show version and updater in the preferences update pane. Update style on the release note link.

Comment on attachment 8862392 [details] Bug 1356507 - Show version and updater in the preferences update pane. https://reviewboard.mozilla.org/r/134340/#review137454 feedback+ from me, looks good! ::: browser/components/preferences/in-content/preferences.xul:40 (Diff revision 2) > <!ENTITY % contentDTD SYSTEM "chrome://browser/locale/preferences/content.dtd"> > <!ENTITY % applicationsDTD SYSTEM > "chrome://browser/locale/preferences/applications.dtd"> > <!ENTITY % advancedDTD SYSTEM > "chrome://browser/locale/preferences/advanced.dtd"> > +<!ENTITY % aboutDialogDTD SYSTEM "chrome://browser/locale/aboutDialog.dtd" > Please add a note in aboutDialog.dtd that these strings are used from two different places. Flod, is this usage OK with you?

(regarding question in comment 21)

... and bug 1330121 needs to land first.

(In reply to (Away 3 May - 5 May) Jared Wein [:jaws] (please needinfo? me) from comment #22) > (regarding question in comment 21) Thanks for flagging. I've checked and it's over 20 strings that we would need to port, so I'm OK with reusing them, especially considering how tricky they are (some locales fail to grasp the concept of 3 concatenated strings). There's one concern though, and that's accesskeys (3 of them). As Jared said, please add a comment to aboutDialog.dtd explaining that some strings are reused in the new preference panel. Also add a similar comment here, explaining that strings from aboutDialog.dtd are displayed in the same section, and that might create accesskey conflicts https://hg.mozilla.org/mozilla-central/file/default/browser/locales/en-US/chrome/browser/preferences/advanced.dtd#l86

Comment on attachment 8862392 [details] Bug 1356507 - Show version and updater in the preferences update pane. https://reviewboard.mozilla.org/r/134340/#review137582 ::: commit-message-5765e:3 (Diff revisions 2 - 3) > -Bug 1356507 - Show version and updater in the preferences update pane. > +Bug 1356507 - Show version and updater in the preferences update pane. r=jaws > > -This quick patch includes unmodified aboutDialog-appUpdater.js into > +This change ncludes unmodified aboutDialog-appUpdater.js into typo: ncludes ::: browser/locales/en-US/chrome/browser/aboutDialog.dtd:7 (Diff revision 3) > - License, v. 2.0. If a copy of the MPL was not distributed with this > - file, You can obtain one at http://mozilla.org/MPL/2.0/. --> > <!ENTITY aboutDialog.title "About &brandFullName;"> > > +<!-- LOCALIZATION NOTE (update.*): > +# Thse strings are used being used in the update pane of preferences also; various typos: "These strings are also used in the update pane of preferences." ::: browser/locales/en-US/chrome/browser/preferences/advanced.dtd:88 (Diff revision 3) > > <!ENTITY updateTab.label "Update"> > > +<!-- LOCALIZATION NOTE (updateApplication.label): > + Strings from aboutDialog.dtd are displayed in this section of the preferences. > + Please be aware of accesskey conflicts. Maybe "Please check for possible accesskey conflicts".

Comment on attachment 8862392 [details] Bug 1356507 - Show version and updater in the preferences update pane. https://reviewboard.mozilla.org/r/134340/#review137740

Is this ready to land now?

(In reply to (Away 3 May - 5 May) Jared Wein [:jaws] (please needinfo? me) from comment #32) > Is this ready to land now? I am waiting for Tina to confirm the UI (comment 11).

Update: Tina and I had a short conversation about this. 1. She would like to have the distribution labels removed. 2. She have problem with the update buttons, which the new pref design need it to be on the right. However that means we will need a new copy string on the left side, which will take even longer to finalize. I will wait for her to make a decision (including the option of leaving the patch as-is).

Tim, thanks for the updates! :) I'll ask the copywriter if we can have her recommendation for the new string. Since the schedule is tight, I'm OK with just moving the button to the right hand side without a new string. I see the new string is related to the content work for Photon, so let's add the new string no later than v57. BTW, Can we add the "Version" in front of the version number as how the spec looks like?

I'm not sure why the distribution labels would be removed and there is no reasoning given in the bug. The distribution labels are helpful if someone is trying to report an issue with Firefox, though I guess that is what about:support is for. We show the distribution labels on the About dialog though, I don't think this bug is where we should make the decision to stop showing them. I would prefer the discussion about hiding the distribution labels be moved to a new bug, and we keep the About dialog and the Updates pane consistent with each other. Moving the button to the right means we should always show the button there. @Tim / @Tina, to prevent having to create new strings right now, can we always show status-related messages as text on the left side, and then actions that require a button on the right side? When a status-related message is unavailable (such as prior to checking for updates), we should just show nothing on the left side. We can also disable the [Check for updates] button while we are checking for updates, downloading an update, or applying an update. For example... Before checking for updates: <blank string> [Check for updates] While checking for updates: Checking for updates... [Check for updates (disabled)] After update has been applied: <blank string> [Restart to update Firefox]

Hey Jared, That makes sense to me. The reason why I suggested hiding the distribution labels was because it looked hard to understand. But I agree that we should be consistent with what Firefox shows in the About dialog. I believe that we can solve the issue by UX/Visual, so let's file a new bug for fixing the complex version details.

The latest revision moves the updater UI mark up and add disabled buttons per comment 36. I've also add the missing space b/t labels and the throbber. Let's go ahead and land this without waiting for further review from :jaws, given that he is away.

Comment on attachment 8862392 [details] Bug 1356507 - Show version and updater in the preferences update pane. https://reviewboard.mozilla.org/r/134340/#review138756 ::: browser/locales/en-US/chrome/browser/preferences/advanced.dtd:89 (Diff revisions 6 - 7) > <!-- LOCALIZATION NOTE (updateApplication.label): > Strings from aboutDialog.dtd are displayed in this section of the preferences. > Please check for possible accesskey conflicts. > --> > <!ENTITY updateApplication.label "&brandShortName; Updates"> > +<!ENTITY updateApplication.version "Version "> Is there a screenshot for the latest iteration? You're concatenating this with another string, which means you need a) a localization note explaining it, so localizers will know why the space is needed b) a "post" string after the version number, that will remain empty in English but not necessarily in other languages.

As asked by Tina, attaching screenshots of the patch to land (and bug 1361640) for her to plan future works.

I will need to address comment 40.

Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2768a8779068 Show version and updater in the preferences update pane. r=jaws

Backed out for eslint failure: https://hg.mozilla.org/integration/autoland/rev/f97d552d92a0529919e9f9957ee5e92dae1d3484 Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=2768a87790681efc0fe64df26c363ef7381759a6&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=96165665&repo=autoland TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/browser/components/preferences/in-content/advanced.js:72:7 | 'gAppUpdater' is not defined. (no-undef) TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/browser/components/preferences/in-content/advanced.js:72:25 | 'appUpdater' is not defined. (no-undef

Sorry. Should be good now.

Comment on attachment 8862392 [details] Bug 1356507 - Show version and updater in the preferences update pane. https://reviewboard.mozilla.org/r/134340/#review138784 ::: browser/locales/en-US/chrome/browser/preferences/advanced.dtd:89 (Diff revision 9) > +<!-- LOCALIZATION NOTE (updateApplication.label): > + Strings from aboutDialog.dtd are displayed in this section of the preferences. > + Please check for possible accesskey conflicts. > +--> > <!ENTITY updateApplication.label "&brandShortName; Updates"> > +<!-- LOCALIZATION NOTE (updateApplication.version.pre): include a trailing space as needed --> Better # LOCALIZATION NOTE (updateApplication.version.*): updateApplication.version.pre # is followed by a version number, keep the trailing space or replace it with a # different character as needed. updateApplication.version.post is displayed # after the version number, and is empty on purpose for English. You can use it # if required by your language.

There are also test failures like this for the previous push: https://treeherder.mozilla.org/logviewer.html#?job_id=96179485&repo=autoland

Thanks for pointing that out. Yeah I can produce that locally although I don't really know what cause it. It's going to take some effort to fix that XPCOM thing. I hope I could find some way to workaround it.

As asked by Tina.

Comment on attachment 8864377 [details] Bug 1356507 - Workaround defineLazyServiceGetter() as described in bug 1361929, https://reviewboard.mozilla.org/r/136056/#review139132 Try looks green, let's try landing it again!

Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/28a9741054a2 Show version and updater in the preferences update pane. r=jaws https://hg.mozilla.org/integration/autoland/rev/7b179818adc2 Workaround defineLazyServiceGetter() as described in bug 1361929, r=mikedeboer

https://hg.mozilla.org/mozilla-central/rev/28a9741054a2 https://hg.mozilla.org/mozilla-central/rev/7b179818adc2

This bug was about to "Updates pane should include current version of Firefox and whether the browser is up-to-date". I have seen the feature being implemented with latest Nightly 55.0a1 on Windows 8.1 (64 bit). This bug's fix is now verified in Latest Nightly. Build ID : 20170517030204 User Agent : Mozilla/5.0 (Windows NT 6.3; WOW64; rv:55.0) Gecko/20100101 Firefox/55.0 [bugday 20170517]

This bug was about "including the current version of Firefox and whether the browser is up-to-date in the Updates pane". I have seen the feature being implemented with latest Nightly on Ubuntu 16.04 LTS ! This bug's fix is now verified in Latest Nightly! Build ID : 20170518100156 User Agent : Mozilla/5.0 (X11; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0 [bugday-20170517]

As per Comment 57 & Comment 58, I am marking this bug as verified fixed.