Closed Bug 1221195 Opened 5 years ago Closed 5 years ago

Home screen requests systemXHR permission for no reason

Categories

(Firefox OS Graveyard :: Gaia::Homescreen, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(b2g-v2.5 fixed)

RESOLVED FIXED
2.6 S1 - 11/20
Tracking Status
b2g-v2.5 --- fixed

People

(Reporter: cwiiis, Assigned: cwiiis)

Details

(Whiteboard: [systemsfe])

Attachments

(1 file, 1 obsolete file)

Just noticed this, and I'm pretty sure we don't use it, let's remove it.
Attachment #8682586 - Flags: review?(gmarty)
Comment on attachment 8682586 [details] [review]
[gaia] Cwiiis:bug1221195-homescreen-remove-system-xhr > mozilla-b2g:master

As discussed offline, it looks good to me.
Attachment #8682586 - Flags: review?(gmarty) → review+
Merged: https://github.com/mozilla-b2g/gaia/commit/6d44ad8956e313e679718df3ac1c393bdd687aec
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Comment on attachment 8682586 [details] [review]
[gaia] Cwiiis:bug1221195-homescreen-remove-system-xhr > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: Not much - I think it'd be nice to merge this so that people know the minimal set of permissions a home screen should apply for, now that we have replaceable home screens.
[Testing completed]: Tested locally
[Risk to taking this patch] (and alternatives if risky): Hopefully zero, but I don't want to speak too soon
[String changes made]: None
Attachment #8682586 - Flags: approval-gaia-v2.5?
I had to revert this in https://github.com/mozilla-b2g/gaia/commit/b6c3b89f2d9ccf8ecc81ba046e5c02f5f7a87067 for breaking a test in gij(22): https://treeherder.mozilla.org/logviewer.html#?job_id=3258999&repo=b2g-inbound

cwiis suggests the test is faulty, but we're reverting this for now to get the tree greened up again.
Flags: needinfo?(chrislord.net)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
apps/system/test/marionette/import_app_test.js fails because it runs code in the homescreen process that requires this permission - A test shouldn't do this, and looking at the test, there doesn't look like there's any reason it does this on the homescreen app and not on the system app (which would make more sense).
Flags: needinfo?(chrislord.net)
Re it making more sense, I'm not sure of that - the activity it's testing is handled by system, so perhaps that's why the code isn't run in system?

Kevin, re comment #6, is there any reason in that test the script is run from the home screen and not from system? Home screen shouldn't need systemXHR, but this test forces its requirement.
Flags: needinfo?(kevingrandon)
Attachment #8682586 - Flags: approval-gaia-v2.5?
Comment on attachment 8683565 [details] [review]
[gaia] Cwiiis:bug1221195-homescreen-remove-system-xhr > mozilla-b2g:master

Rather than needinfo, here's the PR with the amended test.
Flags: needinfo?(kevingrandon)
Attachment #8683565 - Flags: review?(kevingrandon)
Comment on attachment 8683565 [details] [review]
[gaia] Cwiiis:bug1221195-homescreen-remove-system-xhr > mozilla-b2g:master

Carrying r=gmarty
Attachment #8683565 - Flags: review+
Attachment #8682586 - Attachment is obsolete: true
Comment on attachment 8683565 [details] [review]
[gaia] Cwiiis:bug1221195-homescreen-remove-system-xhr > mozilla-b2g:master

Seems fine to me, thanks!
Attachment #8683565 - Flags: review?(kevingrandon) → review+
Thanks, merged: https://github.com/mozilla-b2g/gaia/commit/ccc064c3d32bc9bb6787e768a761055f4fb69769
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Comment on attachment 8683565 [details] [review]
[gaia] Cwiiis:bug1221195-homescreen-remove-system-xhr > mozilla-b2g:master

Let's try this again :)

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: Good to show the minimal set of permissions a home screen needs for users who are interested in developing a new home screen.
[Testing completed]: Tested locally, seems fine.
[Risk to taking this patch] (and alternatives if risky): Low
[String changes made]: None
Attachment #8683565 - Flags: approval-gaia-v2.5?
Whiteboard: [systemsfe]
Target Milestone: --- → 2.6 S1 - 11/20
Comment on attachment 8683565 [details] [review]
[gaia] Cwiiis:bug1221195-homescreen-remove-system-xhr > mozilla-b2g:master

Approved for 2.5 uplift. 

Thanks
Attachment #8683565 - Flags: approval-gaia-v2.5? → approval-gaia-v2.5+
You need to log in before you can comment on or make changes to this bug.