Closed Bug 919927 Opened 6 years ago Closed 6 years ago

[Permission] use permissions instead of permission/option/options

Categories

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

Other
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:-)

RESOLVED FIXED
blocking-b2g -

People

(Reporter: gasolin, Assigned: gasolin)

References

Details

(Whiteboard: [qa-])

Attachments

(2 files)

Due to recent refactor, Gecko can returns permissions/options attribute to support multiple permissions. the old permission/option attribute will be deprecated.

We should progressively remove current use of permission/option attribute and use permissions/options instead.
Depends on: 898949
In 1.3, permission/option/options are obsolete, use permissions instead

The format will be 

{
  "video-capture": ['back', 'front'],
  "audio-capture": [""]
}

or 

{
  "geolocation":[]
}


the return value will be 

{
  "video-capture": 'back',
  "audio-capture": ""
}
Summary: [Permission] use permissions/options instead of permission/option → [Permission] use permissions instead of permission/option/options
Assignee: nobody → gasolin
Attachment #825713 - Flags: review?(alive)
Attachment #825713 - Flags: review?(alive) → review+
blocking-b2g: --- → 1.3?
(In reply to Fred Lin [:gasolin] from comment #1)
> In 1.3, permission/option/options are obsolete, use permissions instead
> 
> The format will be 
> 
> {
>   "video-capture": ['back', 'front'],
>   "audio-capture": [""]
> }
> 
> or 
> 
> {
>   "geolocation":[]
> }
> 
> 
> the return value will be 
> 
> {
>   "video-capture": 'back',
>   "audio-capture": ""
> }

That format is going to completely break compatibility with the existing apps permissions model if that's the case. The typical format is:

{
  "permissions": {
     "contacts": {
       "description": "",
       "access": "readcreate"
     }
  }
}

Which uses a JSON dictionary, not a list. This is because permissions can have multiple attributes - description, access, etc.
@jsmith, I think there is a misunderstanding that we are not going to change the permission declaration format in manifest.webapp. This bug is for modifying the parameters in mozChromeEvent/mozContentEvent for the permission prompt. It does not have compatibility issue for existing webapps.
(In reply to Shih-Chiang Chien [:schien] from comment #4)
> @jsmith, I think there is a misunderstanding that we are not going to change
> the permission declaration format in manifest.webapp. This bug is for
> modifying the parameters in mozChromeEvent/mozContentEvent for the
> permission prompt. It does not have compatibility issue for existing webapps.

Ah okay - makes sense.
Hi Fred,
The gecko patch is at https://bugzilla.mozilla.org/show_bug.cgi?id=853356. Both patches probably need to be checkin together to avoid break.
Thanks.
Depends on: 853356
merged to gaia-master https://github.com/mozilla-b2g/gaia/commit/5289b66509338239e338851a0258fc4e16a028d2

thanks!
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
No longer blocks: 935316
Blocks: 935557
John - Can you back this out? The associated patch this bug is dependent on was backed out, so right now this implementation ends up causing bug 935557 without the dependency present. As a result, the geolocation prompt is broken right now with this patch present.
Flags: needinfo?(jhford)
Reverted:

[master 6a5cc5d] Revert "Merge pull request #13276 from gasolin/issue-919927"
Status: RESOLVED → REOPENED
Flags: needinfo?(jhford)
Resolution: FIXED → ---
Relanded because this revert is breaking travis.

[master 231840c] Revert "Revert "Merge pull request #13276 from gasolin/issue-919927""
Closing again based on comment 10, although we need to get this backed out cleanly.
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Depends on: 936618
No longer blocks: 935557
Depends on: 935557
Backed out of master by request of jsmith here: https://github.com/mozilla-b2g/gaia/commit/f86f7f5c64eb6fd01ab17f721b542e17ed1c9b7e

An additional commit was required to remove the expected failure of the geolocation test: https://github.com/KevinGrandon/gaia/commit/ff6f5189fc2ed9b5419e4155f1dc6cb9ab32bb3b
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
cherry-pick bug 919927 with bug 935557 fix, make permission manager backward compatible.

feedback? Zac for functional test part
Attachment #829966 - Flags: review?(alive)
Attachment #829966 - Flags: feedback?(zcampbell)
Comment on attachment 829966 [details] [review]
pull request redirect to github 2

F+ for me and a pass on Travis!
Attachment #829966 - Flags: feedback?(zcampbell) → feedback+
Attachment #829966 - Flags: review?(alive) → review+
merged to gaia-master https://github.com/mozilla-b2g/gaia/commit/203307c19afa5894f7cd7f52c5f00812b5c24067

thanks!
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
This is targeted feature for 1.3, not committed, so we don't need to block on this.
blocking-b2g: 1.3? → -
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.