Closed Bug 982182 Opened 6 years ago Closed 6 years ago

Check for Updates in about:apps fails to check for updates

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: myk)

References

Details

Attachments

(2 files, 1 obsolete file)

Manually checking for updates by tapping the Check for Updates item in Tools > Apps doesn't work, either because it has no (apparent) effect or because it opens Marketplace in a new tab instead of checking for updates.

In addition to the user-facing impact, this also makes it difficult to test updates (f.e. bug 970300) as well as fixes to the update code (f.e. bug 970200).
There are three different problems here.  First, there's an occurrence of "WebApps:GetApkVersions" that should be "Webapps:GetApkVersions", which is a regression from bug 958358.  Second, the sendMessageToJava implementation in WebappManager.jsm throws a JSON exception because it wasn't updated to include a GUID in the message when bug 946344 landed.

And finally, if you "Browse the Firefox Marketplace" to open the Marketplace in a new tab, then return to the about:apps tab and touch "Check for Updates", Fennec opens Marketplace in a new tab again instead of checking for updates.

I'll fix the first two bugs here and have filed bug 982548 on the last one.
Blocks: 958358, 946344
Summary: Check for Updates in about:apps does nothing or opens Marketplace → Check for Updates in about:apps fails to check for updates
Note that the patch here was applied on top of the patch for bug 982552, since bug 982552 helped me debug the problem.  And they touch code in the same area.  So if you try to apply this one without applying that one, then application will fail (although the conflict is trivial to resolve).
Depends on: 982552
Attachment #8389688 - Flags: review?(wjohnston) → review?(mark.finkle)
Comment on attachment 8389688 [details] [diff] [review]
patch v1: fix regressions that hang checking for updates for webapps

Does it make sense to lazy load the JSM? It doesn't look like any of the code using sendMessageToJava is called right away, but I might be wrong.
Attachment #8389688 - Flags: review?(mark.finkle) → review+
(In reply to Mark Finkle (:mfinkle) from comment #4)
> Does it make sense to lazy load the JSM? It doesn't look like any of the
> code using sendMessageToJava is called right away, but I might be wrong.

You're right, and it does make sense (plus is cleaner), so I've made this change.  Carrying forward review; this is the version of the patch I'll commit.
Attachment #8389688 - Attachment is obsolete: true
Severity: normal → blocker
The Fx29 version of the fix is even simpler, since only one of the two regressions landed in Fx29.  Here's the trivial, obvious, one-line patch.

[Approval Request Comment]
Bug caused by (feature/regressing bug #):
  bug 958358

User impact if declined:
  Webapp installs won't update.

Testing completed (on m-c, etc.):
  This has landed on inbound but hasn't yet merged to Central.  I did, however,
  test this in my own builds, and fix is obvious and trivial.

Risk to taking this patch (and alternatives if risky):
  The risk is low, and there is no alternative.

String or IDL/UUID changes made by this patch:
  None.
Attachment #8390137 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/0a6f7b7dbeec
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Attachment #8390137 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.