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

RESOLVED FIXED in Firefox 29

Status

()

defect
P1
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: myk, Assigned: mhaigh)

Tracking

({regression})

unspecified
Firefox 30
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox29+ fixed, firefox30 fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

Reporter

Description

5 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

5 years ago
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

5 years ago
Assignee: nobody → mhaigh
Reporter

Comment 2

5 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

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

Comment 4

5 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

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

Comment 6

5 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
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Reporter

Comment 9

5 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

5 years ago
[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.