834462 - There should be more information (e.g. build ID / version number, link to release notes) listed in the notification of a system update in Firefox OS

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, enhancement)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Build Identifier: 20130116230203
Git commit info: 3c84fa2b74b3dbe591f0d4fbedbac9aac...

When I check for updates and if there is an update, in the dropdown of the notification, it specifies "System update" and the size of the update.

Now that Firefox OS 1.0RC1 is sort-of ready, it will be nice to know *what* version / build I'm updating to.

Is it 1.0.0.0? Or 1.0.0.1? Or 1.0.1.0? or 1.1.0.0?

Comment 1

[Dave Hylands [:dhylands]]

There is some additional information presented to gaia in the update-available notification.

I believe that the available information comes from the update.xml file. Our nightlies currently use:

http://update.boot2gecko.org/nightly/update.xml

which looks like (reformatted to make it look nicer here):

<?xml version="1.0"?>
<updates>
  <update
    type="minor"
    appVersion="18.0"
    version="18.0"
    extensionVersion="18.0"
    buildID="20130124070201" 
    licenseURL="http://www.mozilla.com/test/sample-eula.html" 
    detailsURL="http://www.mozilla.com/test/sample-details.html"
  >
    <patch
      type="complete" URL="http://update.boot2gecko.org/nightly/b2g_update_20130124070201.mar?build_id=20130124070201&amp;version=18.0"
      hashFunction="SHA512"
      hashValue="00...79"
      size="51209288"
    />
  </update>
</updates>

buildID seems to be the most useful piece. I'm not sure where the 1.0.0.0 stuff would fit in. There is a field called displayVersion that I see as one of the fields parsed.

The code sets displayVersion to version if displayVersion is missing.

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

So having both the version number and the release notes link present in that notification will likely also be useful, since we have those information seemingly readily available in http://update.boot2gecko.org/nightly/update.xml

Also guessing that this should be in the Gaia component.

Comment 4

[dattaz]

What is status of this ?
We can add info=" information about this update" in update.xml
and gaia should show content is info from update.xml

Comment 5

[:gerard-majax]

(In reply to dattaz from comment #4)
> What is status of this ?
> We can add info=" information about this update" in update.xml
> and gaia should show content is info from update.xml

Dattaz, before working on exposing strings to the user, maybe you can start by just exposing the versions/build infos when we get the update prompt ?

Comment 6

[:gerard-majax]

Gabriele and Kai, I think this is the kind of feature that partners may want to have.

Comment 7

[Kai-Zhen Li [:seinlin][:kli]]

Yes, our partners discussed this kind of feature with me.

Comment 8

[Gabriele Svelto [:gsvelto] (PTO)]

Clearing my NI, since Kai-Zhen already replied. BTW Kai-Zhen will you post a summary of what they might need here once you're done gathering their requirements?

Comment 9

[:gerard-majax]

Including in polish. While working on bug 1102959 I've noticed that the mozChromeEvent sent by Gecko does indeed contains some (maybe all?) of the XML field. So we could easily augment the dialog.

Comment 10

[:gerard-majax]

Stephany, what's your point of view regarding comment 9 ?

Comment 11

[Stephany Wilkes]

I'm the UX Program Manager and am not the UX person assigned to this area. Flagging Rob as he worked on this bug previously.

Comment 12

[:gerard-majax]

I'm going to start hacking a new dialog exposing those infos.

Comment 13

[GitHub Autolander Bot]

Attachment: [gaia] lissyx:bug834462 > mozilla-b2g:master

Comment 14

[:gerard-majax]

Attachment: Capture du 2015-04-16 17:45:36.png

Quickly hacked UI. We need to check with UX:
 - which properties do we want to display, and how
 - how can we display release notes

And also, can we make use of the detailsURL for those release notes?

Comment 15

[:gerard-majax]

Robert, I see you have a long history of hacking and reviewing the code in toolkit/mozapps/update/nsUpdateService.js so you can probably help me answer this question: can we make use of detailsURL field to link to and display some release notes for a build ?

Comment 16

[Robert Strong (they/them - no direct email)]

Definitely... it has been used for exactly that in the distant past.

Keep in mind that it can be specified in the update.xml and if it isn't specified there the app.update.url.details pref will be checked for a value.

Comment 17

[:gerard-majax]

Attachment: Capture du 2015-04-20 11:35:54.png

Comment 18

[:gerard-majax]

Adam, this is something that can be interesting for you. I'm also tagging this as foxfood this it might make a lot of sense in this context.

Comment 20

[Kai-Chih Hu [:khu]]

(In reply to Stephany Wilkes from comment #11)
> I'm the UX Program Manager and am not the UX person assigned to this area.
> Flagging Rob as he worked on this bug previously.

This needinfo is pending for several months. Needinfo [e-mail] here.

Comment 21

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Harly/Rob,
Do we have UX resource assigned for the "update notification" ?
This feature is included in 2.5 scope.

Comment 22

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Hi Jacqueline,
Are you on top of this?

Comment 23

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Hey Gregor, 
Could you find someone to take this bug?

Comment 24

[Gregor Wagner [:gwagner]]

As discussed via email, we don't have anyone that can take this bug atm.

Comment 25

[Jacqueline Savory [:jsavory] UX]

I’ve provided a link to the latest OTA spec here as well as in the user story section. I believe that this bug should be covered in the system update dialog variations on page 10. 

Let me know if this covers all of the components we want to include in the update dialogue. If possible, it would be helpful to include a simplified list of features available in the update (screen 3 p.10) but this is only suggested if we can provide useful, concise information. 

Let me know if there is any feedback or concerns, here is the link:
https://mozilla.box.com/s/j9dsl6ilwvqvc9s1mt2dzw33xte87gpp

Comment 26

[:gerard-majax]

(In reply to Jacqueline Savory [:jsavory] UX from comment #25)
> I’ve provided a link to the latest OTA spec here as well as in the user
> story section. I believe that this bug should be covered in the system
> update dialog variations on page 10. 
> 
> Let me know if this covers all of the components we want to include in the
> update dialogue. If possible, it would be helpful to include a simplified
> list of features available in the update (screen 3 p.10) but this is only
> suggested if we can provide useful, concise information. 
> 
> Let me know if there is any feedback or concerns, here is the link:
> https://mozilla.box.com/s/j9dsl6ilwvqvc9s1mt2dzw33xte87gpp

Jacqueline, thanks for providing this. IMHO it fills parts of the needs. I don't really know how we can commit to a list of feature, either generate it automatically or by hand (meta bugs closed?) but this should be something workable and it's only a big deal in the case of dogfooders with frequent updates. It's less of a problem when we are talking about big releases, since we can easily prepare a nice changelog.

So I think the proposed spec just lacks build informations (version and ID). Thanks!

Comment 27

[Gabriele Svelto [:gsvelto] (PTO)]

I'm working on this, taking Alexandre's patch as the starting point.

Comment 28

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: Mock of the update list

After having studied the IxD doc a bit I've come up with a mock that doesn't use a separate panel but rather inserts the build ID (and optionally the link to the release notes) in the list. I'd like to have a visual design before proceeding further. In the mock I've just reused a lighter font weight for the build ID (with the same font size and box alignment) and the link has our usual cyanish color. Since I don't know if this is appropriate please feel free to correct it.

Comment 29

[Patryk Adamczyk [:patryk] UX]

Comment on attachment 8665391 [details]
Mock of the update list

The fonts look fine. 
However the left and right margins should be even. It looks like the check boxes on the right are too far in, they should be aligned with the right edge of the "download" button.

Comment 30

[Gabriele Svelto [:gsvelto] (PTO)]

Thanks for the feedback Patryk, is the color of the link alright too? I haven't modified the check boxes, that's their current positioning you see in the screenshot. I can fix them as part of this bug or open another one if you prefer.

Comment 31

[Gabriele Svelto [:gsvelto] (PTO)]

BTW I've just noticed another issue with the menu, the actual menu area is a slightly darker shade of grey than the background.

Comment 32

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: Mock of the update list v2

Here's a new screenshot with both sides of the list having the same padding. The right side had a different padding due to the RTL rules used for the start padding. Note that I didn't align the checkboxes with the right side of the "Download" button because the text on the left isn't aligned with the left edge of the "Later" button either and I think that was intended. Padding on both sides is now symmetric though.

Comment 33

[GitHub Autolander Bot]

Attachment: [gaia] gabrielesvelto:bug-834462-update-dialog > mozilla-b2g:master

Comment 34

[Gabriele Svelto [:gsvelto] (PTO)]

Comment on attachment 8666726 [details] [review]
[gaia] gabrielesvelto:bug-834462-update-dialog > mozilla-b2g:master

Here's a patch that adds the buildID and release news link to the list plus the necessary adjustments. A few remarks:

- I'm displaying a different string if there's only a system update available versus one or more regular app updates (as per the UX spec).

- The "View the full list of changes" link is not shown if there's no detailsURL field in the update manifest or if it points to about:blank (which apparently it does on some of our OTA servers).

- No tests yet, I'd like to finalize the approach before writing them.

- Besides doing the necessary CSS adjustments I've fixed a couple of issues that were introduced with the RTL changes. These correct the padding of the checkboxes on the right in LTR mode and fix the position of the labels in RTL mode (that float: "left" directive was messing up the whole layout in RTL mode). I'm not sure if that's the right way to handle the padding though, I'll have to check the RTL guidelines to see if there's a better one.

- The methods in the UpdateManager object are *huge* even without my changes, maybe it would be worth splitting them up a bit.

Comment 35

[Etienne Segonzac (:etienne)]

Comment on attachment 8666726 [details] [review]
[gaia] gabrielesvelto:bug-834462-update-dialog > mozilla-b2g:master

Wow that's pretty old code :)
But the changes look great! Ready for unit-testing.

And yes, the dom generation could be extracted quite nicely if you feel like doing the extra changes in this path (but can be a follow up too! would be a nice mentored bug).

Comment 36

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to Etienne Segonzac (:etienne) from comment #35)
> Wow that's pretty old code :)
> But the changes look great! Ready for unit-testing.

Great, I'll put up the tests ASAP.

> And yes, the dom generation could be extracted quite nicely if you feel like
> doing the extra changes in this path (but can be a follow up too! would be a
> nice mentored bug).

My thought exactly :)

Comment 37

[Gabriele Svelto [:gsvelto] (PTO)]

Comment on attachment 8666726 [details] [review]
[gaia] gabrielesvelto:bug-834462-update-dialog > mozilla-b2g:master

Now with unit-tests added. I've noticed another thing which would be worthy of a follow-up: the UpdateManager unit-tests are calling MockNavigatorMozMobileConnections.mTeardown() in the suiteTeardown() function instead of the teardown() one. This is causing certain tests to pass even though they shouldn't as mock connections are being leaked from one test to the next one. I think it's worth investigating it as the failures might just be benign (i.e. only the test setup is wrong) or they could have hidden some real problems.

Comment 38

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: Screenshot of the patch in LTR mode

Comment 39

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: Screenshot of the patch in RTL mode

Comment 40

[Etienne Segonzac (:etienne)]

Comment on attachment 8666726 [details] [review]
[gaia] gabrielesvelto:bug-834462-update-dialog > mozilla-b2g:master

r=me with the test added, thanks!

Comment 41

[Gabriele Svelto [:gsvelto] (PTO)]

Thanks for the review! I've pushed a new version with the added unit-test and will be waiting for the UI review before merging.

Comment 42

[Gabriele Svelto [:gsvelto] (PTO)]

Merged to gaia/master c59d50f033e44c655460f1a7db70281c2cc4cd00

https://github.com/mozilla-b2g/gaia/commit/c59d50f033e44c655460f1a7db70281c2cc4cd00