Closed
Bug 816900
Opened 12 years ago
Closed 11 years ago
[Permission] askPermission() from PermissionPromptHelpter.jsm calls expandPermissions() but no meaning.
Categories
(Firefox OS Graveyard :: General, defect)
Tracking
(firefox19 wontfix, firefox20 wontfix, firefox21 fixed)
RESOLVED
FIXED
People
(Reporter: mchen, Assigned: bechen)
References
Details
Attachments
(1 file, 4 obsolete files)
In askPermission(), it use |access="readwrite"| to do expandPermissions(), this returns an arry as [contacts-read, contacts-write and contacts-create]. But askPermission() just test a permission combined by |msg.type + "-" + msg.access| (ex:contacts-write). So it will take this single permission be checked three times (line 71).
Assignee | ||
Comment 1•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Attachment #697808 -
Flags: feedback?(anygregor)
Comment 2•12 years ago
|
||
Comment on attachment 697808 [details] [diff] [review] quick fix I think you can also hoist the first lines out of the loop. Thanks!
Attachment #697808 -
Flags: feedback?(anygregor) → feedback+
Updated•12 years ago
|
Assignee: nobody → bechen
Assignee | ||
Comment 3•12 years ago
|
||
Attachment #697808 -
Attachment is obsolete: true
Attachment #697841 -
Flags: review?(anygregor)
Updated•12 years ago
|
Attachment #697841 -
Flags: review?(anygregor) → review+
Assignee | ||
Comment 4•12 years ago
|
||
version 2: fix marionette test fail "ERROR - TEST-UNEXPECTED-FAIL | test_sim_contacts.js | ScriptTimeoutException: timed out"
Attachment #697841 -
Attachment is obsolete: true
Attachment #699064 -
Flags: review?(htsai)
Comment 5•12 years ago
|
||
Comment on attachment 699064 [details] [diff] [review] 1. Use |expandedPermNames[idx]| instead |msg.type + "-" + msg.access|. 2. Add |write, create| permissions in test_sim_contacts.js Review of attachment 699064 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/contacts/tests/marionette/test_sim_contacts.js @@ +4,5 @@ > MARIONETTE_TIMEOUT = 30000; > > SpecialPowers.addPermission("contacts-read", true, document); > +// TODO: see bug 804623, We are preventing "read" operations > +// even if just "write" has been set to DENY_ACTION nit: place a period at the end of a line. @@ +5,5 @@ > > SpecialPowers.addPermission("contacts-read", true, document); > +// TODO: see bug 804623, We are preventing "read" operations > +// even if just "write" has been set to DENY_ACTION > +// bug 816900: Because PermissionPromptHelper.jsm will expand the contacts-read nit: s/bug/Bug @@ +7,5 @@ > +// TODO: see bug 804623, We are preventing "read" operations > +// even if just "write" has been set to DENY_ACTION > +// bug 816900: Because PermissionPromptHelper.jsm will expand the contacts-read > +// to contacts-read, contacts-write, contacts-create, > +// we add the other two permissions here nit: white spaces & don't forget a period. Please wrap the sentences at the 80th character. @@ +14,1 @@ > Remember to remove those permissions in cleanUp() function.
Attachment #699064 -
Flags: review?(htsai)
Assignee | ||
Comment 6•12 years ago
|
||
Attachment #699064 -
Attachment is obsolete: true
Attachment #699544 -
Flags: review?(htsai)
Comment 7•12 years ago
|
||
Comment on attachment 699544 [details] [diff] [review] 1. Use |expandedPermNames[idx]| instead |msg.type + "-" + msg.access|. 2. Add |write, create| permissions in test_sim_contacts.js Review of attachment 699544 [details] [diff] [review]: ----------------------------------------------------------------- Nice! Thanks, but please take care of the nits below. r=me for the part of test_sim_contacts.js. ::: dom/contacts/tests/marionette/test_sim_contacts.js @@ +6,5 @@ > SpecialPowers.addPermission("contacts-read", true, document); > +// TODO: see bug 804623, We are preventing "read" operations > +// even if just "write" has been set to DENY_ACTION. > +// Bug 816900: Because PermissionPromptHelper.jsm will expand the contacts-read > +// to contacts-read, contacts-write, contacts-create, nit: remove the white space at the end of the line. @@ +7,5 @@ > +// TODO: see bug 804623, We are preventing "read" operations > +// even if just "write" has been set to DENY_ACTION. > +// Bug 816900: Because PermissionPromptHelper.jsm will expand the contacts-read > +// to contacts-read, contacts-write, contacts-create, > +// we add the other two permissions here nit: remove the white space at the end of the line & place a period.
Attachment #699544 -
Flags: review?(htsai) → review+
Assignee | ||
Comment 8•12 years ago
|
||
r+ after addressing comment 7.
Attachment #699544 -
Attachment is obsolete: true
Attachment #699568 -
Flags: review+
Assignee | ||
Comment 9•11 years ago
|
||
try server https://tbpl.mozilla.org/?tree=Try&rev=aaf23490f063
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 10•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d08a08f79c3 Please remember to type the bug number and the r=foo in the commit message next time. ;)
Keywords: checkin-needed
Comment 11•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3d08a08f79c3
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 12•11 years ago
|
||
Should we further ask this to be checked in into the b2g18 branch?
Flags: needinfo?(anygregor)
Comment 13•11 years ago
|
||
(In reply to Gene Lian [:gene] from comment #12) > Should we further ask this to be checked in into the b2g18 branch? It might be to late now but you can ask for approval.
Flags: needinfo?(anygregor)
Comment 14•11 years ago
|
||
Hi Benjamin, it's up to you to decide if we need to check in this into b2g18. If you want, please ask for approval. Thanks!
Comment 15•11 years ago
|
||
(In reply to Gene Lian [:gene] from comment #14) > Hi Benjamin, it's up to you to decide if we need to check in this into > b2g18. If you want, please ask for approval. Thanks! FYI - Please read https://groups.google.com/forum/#!topic/mozilla.dev.b2g/RhE99AtyTPE. Only tef+ bugs now can be checked into b2g18.
You need to log in
before you can comment on or make changes to this bug.
Description
•