Last Comment Bug 776664 - Check capabilities for SMS access
: Check capabilities for SMS access
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla17
Assigned To: Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
:
:
Mentors:
Depends on:
Blocks: 776652
  Show dependency treegraph
 
Reported: 2012-07-23 13:08 PDT by Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
Modified: 2012-08-17 05:29 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+


Attachments
Check "sms" capability for PSms (819 bytes, patch)
2012-07-24 01:57 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Splinter Review
Check "sms" capability for PSms, v2 (1.23 KB, patch)
2012-07-25 02:49 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Splinter Review
Check "sms" capability for PSms, v3 (1.16 KB, patch)
2012-07-26 16:53 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
justin.lebar+bug: review+
Details | Diff | Splinter Review

Description Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-07-23 13:08:37 PDT

    
Comment 1 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-07-24 01:57:10 PDT
Created attachment 645228 [details] [diff] [review]
Check "sms" capability for PSms

This patch doesn't work currently because sms isn't hooked up to the permission manager.  (I get UNKNOWN_ACTION when running this.)  But cross-process sms also crashes, so this doesn't break things any further.

I'm also assuming here that we want a coarse-grained "access all of the SMS API" permission, not something finer-grained like reading/writing.  If that assumption is wrong, let me know.
Comment 2 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-07-25 02:49:37 PDT
Created attachment 645686 [details] [diff] [review]
Check "sms" capability for PSms, v2

Same assumption applies.
Comment 3 Mounir Lamouri (:mounir) 2012-07-25 11:22:26 PDT
Comment on attachment 645686 [details] [diff] [review]
Check "sms" capability for PSms, v2

Review of attachment 645686 [details] [diff] [review]:
-----------------------------------------------------------------

I would prefer to review that when all the other plumbing will be done (ie. capabilities and sms permissions). r+ a patch that doesn't work/isn't testable doesn't make that much sense. Even more when it's that trivial ;)
Comment 4 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-07-26 13:54:30 PDT
I have a working patch built on bug 776649 that I've been testing with the last two days, but will hold off posting per mounir's request.  Doesn't really matter, it's a trivial patch.
Comment 5 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-07-26 16:53:11 PDT
Created attachment 646414 [details] [diff] [review]
Check "sms" capability for PSms, v3
Comment 6 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-07-27 13:48:14 PDT
Comment on attachment 646414 [details] [diff] [review]
Check "sms" capability for PSms, v3

Hope you don't mind pinch-reviewing for mounir here, but this patch is deliciously trivial.
Comment 7 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-16 12:35:18 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/769750c602b2
Comment 8 Ed Morley [:emorley] 2012-08-17 05:29:50 PDT
https://hg.mozilla.org/mozilla-central/rev/769750c602b2

Note You need to log in before you can comment on or make changes to this bug.