Closed
Bug 1060278
Opened 10 years ago
Closed 10 years ago
Missing strings for implicit permission names
Categories
(Firefox OS Graveyard :: Gaia::Settings, defect)
Tracking
(blocking-b2g:2.1+, b2g-v2.1 fixed, b2g-v2.2 fixed)
People
(Reporter: freddy, Assigned: freddy)
References
Details
(Keywords: late-l10n)
Attachments
(1 file, 1 obsolete file)
46 bytes,
text/x-github-pull-request
|
freddy
:
review+
bajaj
:
approval-gaia-v2.1+
|
Details | Review |
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.
Assignee | ||
Comment 1•10 years ago
|
||
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.
Assignee | ||
Comment 3•10 years ago
|
||
:flod said on IRC that re-ordering is fine. I'll try to take this.
Assignee: nobody → fbraun
Assignee | ||
Comment 4•10 years ago
|
||
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?
Assignee | ||
Comment 5•10 years ago
|
||
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?
Assignee | ||
Comment 6•10 years ago
|
||
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 7•10 years ago
|
||
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)
Assignee | ||
Comment 8•10 years ago
|
||
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)
Assignee | ||
Comment 9•10 years ago
|
||
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 10•10 years ago
|
||
Comment on attachment 8481192 [details] [review] work in progress Redirecting r? to flod.
Attachment #8481192 -
Flags: review?(gandalf) → review?(francesco.lodolo)
Comment 11•10 years ago
|
||
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+
Assignee | ||
Comment 12•10 years ago
|
||
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)
Assignee | ||
Comment 13•10 years ago
|
||
Comment on attachment 8482281 [details] [review] new attempt Oops, I didn't see r+ from gasolin. carrying over.
Attachment #8482281 -
Flags: review?(gasolin) → review+
Comment 14•10 years ago
|
||
(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 15•10 years ago
|
||
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)
Assignee | ||
Comment 16•10 years ago
|
||
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
Comment 17•10 years ago
|
||
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)
Comment 18•10 years ago
|
||
[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?
Updated•10 years ago
|
blocking-b2g: 2.1? → 2.1+
Assignee | ||
Comment 19•10 years ago
|
||
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?
Assignee | ||
Comment 20•10 years ago
|
||
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
Updated•10 years ago
|
Attachment #8482281 -
Flags: approval-gaia-v2.1? → approval-gaia-v2.1+
Comment 21•10 years ago
|
||
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
status-b2g-v2.1:
--- → fixed
status-b2g-v2.2:
--- → fixed
Comment 22•10 years ago
|
||
(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.
Description
•