Closed Bug 1060278 Opened 10 years ago Closed 10 years ago

Missing strings for implicit permission names

Categories

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

x86_64
Linux
defect
Not set
normal

Tracking

(blocking-b2g:2.1+, b2g-v2.1 fixed, b2g-v2.2 fixed)

RESOLVED FIXED
2.1 S4 (12sep)
blocking-b2g 2.1+
Tracking Status
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed

People

(Reporter: freddy, Assigned: freddy)

References

Details

(Keywords: late-l10n)

Attachments

(1 file, 1 obsolete file)

Bug 1049371 brought a developer setting that allows showing more permissions in the App Permission panel of the settings app.

Vishy found out that I have left out some permission names, which need to be added to 'shared/locales/permissions/permissions.en-US.properties'. Vishy mentioned |tcp-sockets|, but I'm not confident I that this is the only omission.
Maybe the permission properties file should be updated with an alphabetical list of all permissions. This would improve syncing and navigating through the list of strings.
-> Settings bug
Component: Gaia::L10n → Gaia::Settings
:flod said on IRC that re-ordering is fine. I'll try to take this.
Assignee: nobody → fbraun
Sorting the list in my first commit. Then adding permissions that I found missing.
I noticed that the appRequest/webRequest properties contain the description text for a setting of Prompt (i.e.. app/site "would like to access your videos".

Considering that the debug feature allows setting everything (privileged) to prompt - does that mean we need an appRequest string for all permissions in this file?
Attached file work in progress (obsolete) —
First commit just sorts the permission names alphabetically.
The second commit adds missing names (extracted from PermissionTable.jsm) and tries to find good strings for it.

It contains a bit of technical jargon, which may not be super helpful for end users. Feedback would be really appreciated.
Attachment #8481192 - Flags: feedback?
Comment on attachment 8481192 [details] [review]
work in progress

Fred, can you take a look?
2.1 string freeze is tomorrow and I'd like to get this landed before.
Attachment #8481192 - Flags: feedback? → feedback?(gasolin)
Comment on attachment 8481192 [details] [review]
work in progress

I saw several un-certain comments and strings in PR. And I prefer to keep the origin orders but put developer only strings at the bottom.

The order like perm-fmradio should be the first string before perm-fmradio-xx strings.
Attachment #8481192 - Flags: feedback?(gasolin)
Comment on attachment 8481192 [details] [review]
work in progress

I redid the sorting so that perm-foo comes before perm-foo-apprequest, -webrequest etc.
Then I pushed another commit that moves all developer strings to the bottom, to make a clear distinction.

Can you please take another look?
Attachment #8481192 - Flags: review?(gasolin)
Comment on attachment 8481192 [details] [review]
work in progress

About the uncertain strings: I wanted to get another review from l10n to make sure they make sense. The plan was to flag 'review?' from gandalf, but I guess there's no harm in flagging this in parallel :-)
Attachment #8481192 - Flags: review?(gandalf)
Comment on attachment 8481192 [details] [review]
work in progress

Redirecting r? to flod.
Attachment #8481192 - Flags: review?(gandalf) → review?(francesco.lodolo)
Comment on attachment 8481192 [details] [review]
work in progress

Thanks for your patch. overall looks good to me.

please address nits mentioned in github and remove those `unsure` comments.
Attachment #8481192 - Flags: review?(gasolin) → review+
Attached file new attempt
Apologies for the review-mess. This branch has a cleaner git history:

the first commit (b53db14) just sorts the existing IDs.
the second commit adds new string IDs, that I would like to have reviewed by you, Matej (5c37b3c)


The last commit removes the comments that Fred suggested I remove.
Attachment #8481192 - Attachment is obsolete: true
Attachment #8481192 - Flags: review?(francesco.lodolo)
Attachment #8482281 - Flags: review?(gasolin)
Attachment #8482281 - Flags: review?(Mnovak)
Comment on attachment 8482281 [details] [review]
new attempt

Oops, I didn't see r+ from gasolin. carrying over.
Attachment #8482281 - Flags: review?(gasolin) → review+
(In reply to Frederik Braun [:freddyb] from comment #12)
> Created attachment 8482281 [details] [review]
> new attempt
> 
> Apologies for the review-mess. This branch has a cleaner git history:
> 
> the first commit (b53db14) just sorts the existing IDs.
> the second commit adds new string IDs, that I would like to have reviewed by
> you, Matej (5c37b3c)
> 
> 
> The last commit removes the comments that Fred suggested I remove.

I don't normally do reviews like these. Can someone clarify what you need me to look at? Thanks.
Comment on attachment 8482281 [details] [review]
new attempt

Had a chat with freddyb on irc. Since these are aimed at developers, I don't need to review them. Thanks.
Attachment #8482281 - Flags: review?(Mnovak)
Thanks for the clarification Matej. I will go on without a review then.
A new commit incorporates Fred's suggested changes. I have rebased and squashed it into one commit.
Please check in.
Keywords: checkin-needed
https://github.com/mozilla-b2g/gaia/commit/899d9ca2ca77a7dd8895ddf5b2567109f7418cd4
Status: NEW → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S4 (12sep)
[Blocking Requested - why for this release]:This bug fixes the issue of not showing the names of certain permissions in the verbose mode. Since the feature is introduced in 2.1, it makes sense to also fix this bug in the same release.
blocking-b2g: --- → 2.1?
blocking-b2g: 2.1? → 2.1+
Comment on attachment 8482281 [details] [review]
new attempt

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): 1049371
[User impact] if declined: Verbose app permissions (requires a developer pref to be visibles) will show as select boxes with no title.
[Testing completed]: No tests, just new strings.
[Risk to taking this patch] (and alternatives if risky): -
[String changes made]: This adds strings, they are not visible to normal users. This is all behind a developer settings. That's also why they contain technical jargon.
Attachment #8482281 - Flags: approval-gaia-v2.1?
Since I couldn't check this in myself, this just barely missed the 2.1 branch. So this also missed the string freeze :/
Keywords: late-l10n
Attachment #8482281 - Flags: approval-gaia-v2.1? → approval-gaia-v2.1+
String freeze was extended to 14-Sep per Bhavana's email to dev-b2g/gaia on 2-Sep. Please give it a read if you haven't already done so as it contains a lot of good information.

v2.1: https://github.com/mozilla-b2g/gaia/commit/a2393917a358a616950d0b02280e03c5c10795b9
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #21)
> String freeze was extended to 14-Sep per Bhavana's email to dev-b2g/gaia on
> 2-Sep. Please give it a read if you haven't already done so as it contains a
> lot of good information.

I object. String freeze is already in place (we accept some strings if necessary), 'hard' string freeze will be Sep 14.
That doesn't mean that any string is allowed to land from now to Sep 14, quite the contrary.

https://groups.google.com/forum/#!topic/mozilla.dev.gaia/h0VmL8cpr8M
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: