970209 - webapp update logic in about:apps should be #ifdef MOZ_ANDROID_SYNTHAPKS

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Web Apps (PWAs), defect, P1)

Description

[Myk Melez [:myk] [@mykmelez]]

The code in about:apps that enables users to manually check for updates is not #ifdef MOZ_ANDROID_SYNTHAPKS, although it is specific to synthetic APKs and does not work with the old implementation.  So it should depend on the new implementation being specified.

Comment 1

[Martyn Haigh (:mhaigh)]

Attachment: Surround update logic in about:apps with synth apk specific wrapper

Pretty sure I got it all, but may need a trained eye to go over the rest of the code to make sure.

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 8374135 [details] [diff] [review]
Surround update logic in about:apps with synth apk specific wrapper

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

This looks like the patch for bug 967735!

Comment 3

[Martyn Haigh (:mhaigh)]

Attachment: Surround update logic in about:apps with synth apk specific wrapper

Ahem - this is the correct patch!

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 8374804 [details] [diff] [review]
Surround update logic in about:apps with synth apk specific wrapper

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

Good start, but this also needs to ifdef the update-item in aboutApps.xhtml!

Comment 5

[Martyn Haigh (:mhaigh)]

Attachment: Surround update logic in about:apps with synth apk specific wrapper

updated html file

Comment 6

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 8377133 [details] [diff] [review]
Surround update logic in about:apps with synth apk specific wrapper

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

Looks good!

Comment 7

[Myk Melez [:myk] [@mykmelez]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/606b524f6299

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/606b524f6299

Comment 9

[Myk Melez [:myk] [@mykmelez]]

Martyn, can you request uplift to Beta for this change?

To do so, first create a patch that applies cleanly to the Beta branch, which contains a trivial conflict with the existing patch due to the fix for bug 982885.

Then attach the patch to this bug, selecting "?" from the approval-mozilla-beta dropdown menu in the "add an attachment" form, and filling out the "approval request" form that automagically appears in the Comment field.  See bug 982559, comment 11 for an example of a verbose uplift request and bug 958709, comment 19 for a succinct one.

Comment 10

[Martyn Haigh (:mhaigh)]

Attachment: Surround update logic in about:apps with synth apk specific wrapper

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 982885
User impact if declined: Users of Fennec without Synthetic-APK enabled will be able to manually check for updates which will not work
Testing completed (on m-c, etc.):  This patch landed on mozilla-central a while ago and hasn't caused any regressions.
Risk to taking this patch (and alternatives if risky): None
String or IDL/UUID changes made by this patch: None

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3672b5508f0e