The app status is not set correctly for hosted certified apps

RESOLVED FIXED in Firefox 40

Status

RESOLVED FIXED
3 years ago
10 months ago

People

(Reporter: amac, Assigned: amac)

Tracking

unspecified
mozilla40
x86_64
Windows 8.1

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

3 years ago
STR: 
1. Enable developer mode (set 
pref("dom.apps.developer_mode", true);)
2. Boot and install any hosted app that is certified
3. Check the app status at /data/local/webapps/webapps.json

EXPECTED: 

The appStatus should be 3 (Certified)

ACTUAL:

The appStatus is 1. And if the app uses web components on any other thing that explicitly check for the status, it doesn't work.
(Assignee)

Comment 1

3 years ago
Created attachment 8593435 [details] [diff] [review]
V1. Set the correct app status for hosted apps on developer mode
Assignee: nobody → amac.bug
Attachment #8593435 - Flags: review?(fabrice)
Comment on attachment 8593435 [details] [diff] [review]
V1. Set the correct app status for hosted apps on developer mode

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

::: dom/apps/Webapps.jsm
@@ +3107,5 @@
>      // don't update the permissions yet.
>      if (!aData.isPackage) {
>        if (supportUseCurrentProfile()) {
> +        // Until now we were not setting correctly the appStatus for hosted apps, and that didn't matter because
> +        // hosted apps couldn't be other than installed anyway. That's not true anymore...

Remove this comment. Historians will look at the blame & bugzilla.
Attachment #8593435 - Flags: review?(fabrice) → review+
Blocks: 1111961
(Assignee)

Comment 3

3 years ago
Created attachment 8593807 [details] [diff] [review]
V2, with review comments adressed

r=fabrice

Try run at:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae2d76e6200a
Attachment #8593807 - Flags: review+
(Assignee)

Updated

3 years ago
Attachment #8593435 - Attachment is obsolete: true
(Assignee)

Comment 4

3 years ago
Test run looks (mostly) green, requesting checkin.
Keywords: checkin-needed
Your B2G/Mulet test failures look legit...
213 INFO TEST-UNEXPECTED-FAIL | dom/apps/tests/test_app_enabled.html | Test timed out. - expected PASS
Keywords: checkin-needed
(Assignee)

Comment 6

3 years ago
Actually this kinda looks a lot like Bug 1115788. In fact, all of those tests seem to have a lot of oranges judging by the suggestions... The code added here only makes a single change if the developer_mode preference is true. And the only test where that's true is on test_install_dev_mode.html. So for all the rest, the code that's being executed is the same with and without this patch.
Keywords: checkin-needed
The failures aren't happening in production, especially at the high frequency of your test run. Please post a green Try run to verify that your patch isn't at fault rather than asking me to accept it on faith and risk a tree closure if wrong.
Keywords: checkin-needed
(Assignee)

Comment 8

3 years ago
Created attachment 8596561 [details] [diff] [review]
V3. Try-catched the lines to not fail if the preference doesn't exist

r=fabrice
Attachment #8593807 - Attachment is obsolete: true
Attachment #8596561 - Flags: review+
(Assignee)

Comment 10

3 years ago
And it's green... rerequesting checkin
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/83ce3822d127
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40

Updated

10 months ago
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.