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

RESOLVED FIXED

Status

Firefox OS
Gaia::System
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: gasolin, Assigned: gasolin)

Tracking

unspecified
Other
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:-)

Details

(Whiteboard: [qa-])

Attachments

(2 attachments)

(Assignee)

Description

4 years ago
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.
(Assignee)

Updated

4 years ago
Depends on: 898949
(Assignee)

Comment 1

4 years ago
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)

Updated

4 years ago
Assignee: nobody → gasolin
(Assignee)

Comment 2

4 years ago
Created attachment 825713 [details] [review]
pull request redirect to github
Attachment #825713 - Flags: review?(alive)
Attachment #825713 - Flags: review?(alive) → review+
(Assignee)

Updated

4 years ago
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.
(Assignee)

Updated

4 years ago
Depends on: 853356
(Assignee)

Comment 7

4 years ago
merged to gaia-master https://github.com/mozilla-b2g/gaia/commit/5289b66509338239e338851a0258fc4e16a028d2

thanks!
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Blocks: 935316

Updated

4 years ago
No longer blocks: 935316
(Assignee)

Updated

4 years ago
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
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED

Updated

4 years ago
Depends on: 936618

Updated

4 years ago
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 → ---
(Assignee)

Comment 13

4 years ago
Created attachment 829966 [details] [review]
pull request redirect to github 2

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 14

4 years ago
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+
(Assignee)

Comment 15

4 years ago
merged to gaia-master https://github.com/mozilla-b2g/gaia/commit/203307c19afa5894f7cd7f52c5f00812b5c24067

thanks!
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 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? → -

Updated

4 years ago
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.