Closed Bug 970209 Opened 7 years ago Closed 7 years ago

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

Categories

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

ARM
Android
defect

Tracking

()

RESOLVED FIXED
Firefox 30
Tracking Status
firefox29 + fixed
firefox30 --- fixed

People

(Reporter: myk, Assigned: mhaigh)

References

Details

(Keywords: regression)

Attachments

(2 files, 2 obsolete files)

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.
Pretty sure I got it all, but may need a trained eye to go over the rest of the code to make sure.
Attachment #8374135 - Flags: feedback?(myk)
Assignee: nobody → mhaigh
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!
Attachment #8374135 - Flags: feedback?(myk) → feedback-
Ahem - this is the correct patch!
Attachment #8374135 - Attachment is obsolete: true
Attachment #8374804 - Flags: feedback?(myk)
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!
Attachment #8374804 - Flags: feedback?(myk) → feedback+
updated html file
Attachment #8374804 - Attachment is obsolete: true
Attachment #8377133 - Flags: feedback?(myk)
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!
Attachment #8377133 - Flags: review?(wjohnston)
Attachment #8377133 - Flags: feedback?(myk)
Attachment #8377133 - Flags: feedback+
Attachment #8377133 - Flags: review?(wjohnston) → review+
https://hg.mozilla.org/mozilla-central/rev/606b524f6299
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
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.
Flags: needinfo?(mhaigh)
[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
Attachment #8400670 - Flags: approval-mozilla-beta?
Flags: needinfo?(mhaigh)
Attachment #8400670 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.