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

RESOLVED FIXED in Firefox 29

Status

()

Firefox for Android
Web Apps
P1
normal
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: myk, Assigned: mhaigh)

Tracking

({regression})

unspecified
Firefox 30
ARM
Android
regression
Points:
---

Firefox Tracking Flags

(firefox29+ fixed, firefox30 fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

4 years ago
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.
(Assignee)

Comment 1

4 years ago
Created attachment 8374135 [details] [diff] [review]
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.
Attachment #8374135 - Flags: feedback?(myk)
(Assignee)

Updated

4 years ago
Assignee: nobody → mhaigh
(Reporter)

Comment 2

4 years ago
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-
(Assignee)

Comment 3

4 years ago
Created attachment 8374804 [details] [diff] [review]
Surround update logic in about:apps with synth apk specific wrapper

Ahem - this is the correct patch!
Attachment #8374135 - Attachment is obsolete: true
Attachment #8374804 - Flags: feedback?(myk)
(Reporter)

Comment 4

4 years ago
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+
(Assignee)

Comment 5

4 years ago
Created attachment 8377133 [details] [diff] [review]
Surround update logic in about:apps with synth apk specific wrapper

updated html file
Attachment #8374804 - Attachment is obsolete: true
Attachment #8377133 - Flags: feedback?(myk)
(Reporter)

Comment 6

4 years ago
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
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
status-firefox29: --- → affected
status-firefox30: --- → fixed
tracking-firefox29: ? → +
(Reporter)

Comment 9

4 years ago
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)
(Assignee)

Comment 10

4 years ago
Created attachment 8400670 [details] [diff] [review]
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
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.