Closed Bug 1118003 Opened 10 years ago Closed 10 years ago

[Privacy Panel] Include Transparency Control

Categories

(Firefox OS Graveyard :: Gaia, defect)

All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-v2.2 verified, b2g-master verified)

VERIFIED FIXED
2.2 S4 (23jan)
Tracking Status
b2g-v2.2 --- verified
b2g-master --- verified

People

(Reporter: marta, Assigned: kaze)

References

Details

Attachments

(1 file)

47 bytes, text/x-github-pull-request
etienne
: review+
zbraniecki
: review+
Details | Review
This is a working prototype feature. 

As a user I want to have an overview of what apps have which permissions on my phone so I can easily review the permissions I have given each app.
As a user I want to be able to sort the permission view by app vendor, trust level, alphabetical order, my permission name/type  etc. so I can easily review all permissions.

the screenshots can be found at:   https://drive.google.com/a/mozilla.com/folderviewid=0B5rfAsa_Z3pBZUx1RXpUdmNmVFU&usp=sharing#
Assignee: nobody → fabien
Attached file patch proposal
Attachment #8545538 - Flags: review?(etienne)
Comment on attachment 8545538 [details] [review]
patch proposal

First round done!
Attachment #8545538 - Flags: review?(etienne)
Comment on attachment 8545538 [details] [review]
patch proposal

Thanks for this first round Étienne. Here’s an updated version of this patch, with the additional integration tests you suggested.

https://github.com/mozilla-b2g/gaia/pull/27162
Attachment #8545538 - Flags: review?(etienne)
I tried to give the patch a quick test-run but couldn't apply it to current Gaia master.

String-wise the patch looks good to me with the fixes, I'd suggest to ask :gandalf or :stas for feedback on the l10n.js bits just to confirm if everything looks good. 

For example we're trying to move away from using mozL10N.get, while the code seems to rely heavily on it (but as you know it's not my area of expertize)
https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/localization_code_best_practices
The rest of the Privacy Panel relies on mozL10n.get(), too. I’ll propose a follow-up to fix that for the whole app once this patch lands — it is already big enough, imho.
Comment on attachment 8545538 [details] [review]
patch proposal

From a quick glance at the patch, it looks like there are a few places where mozL10n.get is used to assign to textContent.  We're trying to move away from this pattern and use mozL10n.setAttributes instead.   I know Zibi has worked with Marta on the Privacy Panel on these issues before so it probably makes sense for him to take a look at this patch before it lands.  Thanks!
Attachment #8545538 - Flags: review?(gandalf)
I haven’t seen any mozL10n.setAttributes in the Privacy Panel, but I agree there are two places where {HTMLElement}.setAttribute('data-l10n-id', key) could be used instead — in js/applications.js and js/app_details.js.

I’ll update my patch for that in the next hour.
Patch updated.
there is a patch to correct that in the rest of the PP waiting for review
Comment on attachment 8545538 [details] [review]
patch proposal

The l10n part is in good hands, r+ for the rest, kudos on the tests!
Attachment #8545538 - Flags: review?(etienne) → review+
Comment on attachment 8545538 [details] [review]
patch proposal

r+ - Thanks Kaze!

Stas, can you take a look at the permissions list and _orderBy? I think it's an interesting case for how we need a way to get multiple entities together (separate async call would not make sense - sorting only make sense in one locale so it's a make or break scenario).
Attachment #8545538 - Flags: review?(gandalf) → review+
Thanks Zibi. :-)

Commits squashed, ready for merging / uplifting.
Can some try this patch with a multi-locale build? I can't get the panel to localize.

For example I see tc-applications ("Applications") correctly localized in build_stage's json, and even in the app package, but it's displayed in English nonetheless.
Comment on attachment 8545538 [details] [review]
patch proposal

[Approval Request Comment]

[User impact] if declined: lack of an extended view of application permissions — in particular, there’s no user-friendly description of the app permissions, and internal app permissions cannot be known.

[Testing completed]: manual, unit tests, integration tests.

[Risk to taking this patch] (and alternatives if risky): very low — this is a read-only feature, it has absolutely no side-effect on other existing features.

[String changes made]: 15 new UI strings in permissions.en-US.properties + 85 new strings to describe application permissions.
Attachment #8545538 - Flags: approval-gaia-v2.2?
(In reply to Francesco Lodolo [:flod] from comment #14)
> Can some try this patch with a multi-locale build? I can't get the panel to
> localize.

Not sure what changed from the previous multiple attempts but I managed to get it working fine now.
Attachment #8545538 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2?(bbajaj)
FTR: this has been R+’ed before the branching but the tree was closed, so I couldn’t land it in time. :-/
Needs to land on master first. Please seek approval for uplift only after the landings have stuck on master/central.
Sorry Bhavana, I didn’t expect the tree to be closed for so long. Just landed:

https://github.com/mozilla-b2g/gaia/commit/1994b4063e43dbcd1a28b933fea2b7de1e8cfdf2
Flags: needinfo?(bbajaj)
Flags: needinfo?(bbajaj)
Attachment #8545538 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S4 (23jan)
This has been verified as fixed on Flame 3.0

The user can sort the permission view for applications by app vendor, trust level and alphabetical order. 

Environmental Variables:
Device: Flame 3.0(Full Flash)(KK)(319mb)
BuildID: 20150121010204
Gaia: 5e98dc164b17fd6decb48a9eaddef0e55b82e249
Gecko: 540077a30866
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0
Status: RESOLVED → VERIFIED
This is also fixed on Flame 2.2

Transparency control has been implemented in privacy panel and the user can sort the permission view for applications by app vendor, trust level and alphabetical order. 

Environmental Variables:
Device: Flame 2.2(Full Flash)(KK)(319mb)
BuildID: 20150121002607
Gaia: e4f9b5da3751798f9cc5d95f302c30722cc11fca
Gecko: 75a462a58d7a
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0a2 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: