Closed Bug 1524074 Opened 6 years ago Closed 1 month ago

Implement Permissions API extension for Marionette

Categories

(Remote Protocol :: Marionette, enhancement, P2)

Product:
Remote Protocol
Component:
Marionette
Type:
enhancement
Points:
3

Tracking

(firefox128 fixed)

Status:
RESOLVED FIXED
Milestone:
128 Branch
Tracking Flags:
Tracking Status
firefox128 --- fixed

People

(Reporter: ato, Assigned: Sasha)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [webdriver:m11], [wptsync upstream][webdriver:relnote])

Attachments

(2 files, 1 obsolete file)

Bug 1524074 - [marionette] Add validation to "setPermission" API.
48 bytes, text/x-phabricator-request
Details | Review
Bug 1524074 - Enable "marionette.setpermission.enabled" preference for wpt tests.
48 bytes, text/x-phabricator-request
Details | Review
Blocks: webdriver
Priority: -- → P3

James, what's the importance of that given that we seem to fail a lot of wpt tests. See also the comment from Johann over on bug 1671407.

Flags: needinfo?(james)

Ah, apologies, I didn't realise there was already a bug for this.

The question on priorities is really for the people implementing the permissions features. I'd assume that the ability to test those features, and share the tests with Chrome, WebKit and others would be high priority.

Flags: needinfo?(james)

Which other tests beside the following wdspec tests do we have?

Based on rg -l test_driver\..*permiss in testing/web-platform/tests:

screen_enumeration/screen_enumeration_permission.window.js
screen_enumeration/getScreens.values.https.html
screen_enumeration/isMultiScreen.tentative.https.window.js
screen_enumeration/getScreens.tentative.https.window.js
resources/chromium/generic_sensor_mocks.js
resources/chromium/nfc-mock.js
screen-wake-lock/wakelock-enabled-by-feature-policy.https.sub.html
screen-wake-lock/wakelockpermissiondescriptor.https.html
screen-wake-lock/idlharness.https.window.js
screen-wake-lock/wakelock-enabled-on-self-origin-by-feature-policy.https.sub.html
screen-wake-lock/wakelock-onrelease.https.html
screen-wake-lock/wakelock-request-denied.https.html
screen-wake-lock/wakelock-released.https.html
storage-access-api/requestStorageAccess.sub.window.js
idle-detection/idle-detection-allowed-by-feature-policy.https.sub.html
idle-detection/interceptor.https.html
idle-detection/basics.tentative.https.window.js
idle-detection/idle-detection-default-feature-policy.https.sub.html
idle-detection/idle-permission.tentative.https.window.js
idle-detection/idlharness-worker.https.window.js
idle-detection/idle-detection-allowed-by-feature-policy-attribute-redirect-on-load.https.sub.html
idle-detection/idlharness.https.window.js
idle-detection/idle-detection-allowed-by-feature-policy-attribute.https.sub.html
clipboard-apis/async-write-svg-read-svg.https.html
clipboard-apis/async-write-blobs-read-blobs.https.html
clipboard-apis/async-write-html-read-html.https.html
clipboard-apis/async-html-script-removal.https.html
clipboard-apis/async-write-image-read-image.https.html
clipboard-apis/async-svg-script-removal.https.html
web-nfc/nfc_permission.https.html
web-nfc/NDEFWriter_write.https.html
web-nfc/NDEFReader_scan.https.html
mediacapture-image/MediaStreamTrack-getCapabilities.html
mediacapture-image/MediaStreamTrack-applyConstraints.html
mediacapture-image/MediaStreamTrack-getSettings.html
mediacapture-image/MediaStreamTrack-applyConstraints-reject.html
mediacapture-image/MediaStreamTrack-applyConstraints-getSettings.html
generic-sensor/generic-sensor-permission.https.html
feature-policy/resources/feature-policy-screen-wakelock.html
feature-policy/resources/feature-policy-clipboard-write.html
feature-policy/resources/feature-policy-clipboard-read.html
docs/writing-tests/testdriver.md
font-access/resources/test-expectations.js
native-file-system/resources/native-fs-test-helpers.js
clipboard-apis/detached-iframe/writeText-readText-on-detached-iframe.https.html
clipboard-apis/detached-iframe/write-on-detaching-iframe.https.html
clipboard-apis/detached-iframe/write-read-on-detached-iframe.https.html
clipboard-apis/text-write-read/async-writeText-readText.https.html
clipboard-apis/text-write-read/async-writeText-read.https.html
clipboard-apis/detached-iframe/read-on-detaching-iframe.https.html
clipboard-apis/text-write-read/async-write-read.https.html
clipboard-apis/permissions/readText-granted.https.html
clipboard-apis/permissions/writeText-denied.https.html
clipboard-apis/text-write-read/async-write-readText.https.html
clipboard-apis/permissions/writeText-granted.https.html
clipboard-apis/permissions/readText-denied.https.html
fullscreen/api/element-request-fullscreen-options.tentative.html
infrastructure/testdriver/set_permission.https.html
clipboard-apis/feature-policy/clipboard-write/clipboard-write-enabled-on-self-origin-by-feature-policy.tentative.https.sub.html
clipboard-apis/feature-policy/clipboard-write/clipboard-write-disabled-by-feature-policy.tentative.https.sub.html
clipboard-apis/feature-policy/clipboard-write/clipboard-write-enabled-by-feature-policy.tentative.https.sub.html
clipboard-apis/feature-policy/clipboard-read/clipboard-read-enabled-on-self-origin-by-feature-policy.tentative.https.sub.html
clipboard-apis/feature-policy/clipboard-read/clipboard-read-disabled-by-feature-policy.tentative.https.sub.html
clipboard-apis/feature-policy/clipboard-read/clipboard-read-enabled-by-feature-policy.tentative.https.sub.html
Blocks: 1678012

Is this on the DOM radar? I'm trying to figure out what we need to do to make sure we get wpt coverage for these APIs that have additional WebDriver endpoints. Is this work blocked on something specific? Is there help required from the automation people?

Flags: needinfo?(jstutte)
Flags: needinfo?(amarchesini)

With Andrea working on other things I don't think this has an owner in DOM currently. Maybe Johann can help out? (I initially wrote something about the tests listed above, but I did not see that was a scrollable box. It seems our coverage for quite a few APIs could be improved by this.)

Flags: needinfo?(jstutte)
Flags: needinfo?(jhofmann)
Flags: needinfo?(amarchesini)

I don't think anyone on our team can work on this right now, though I will put it on our backlog. :|

Flags: needinfo?(jhofmann)

Right now permissions on Android are handled by the individual app, but not by Gecko itself. It means that we won't actually get full support across all platforms.

But that will change with bug 1654832. Once it's done Gecko itself will handle the permission flags.

Depends on: 1654832

Now with bug 1654832 fixed there should be a way for us to handle permissions in GeckoView. I had a look but didn't found the entry point. Agi, maybe you can give a quick hint in how our code in Marionette could utilize the GV permissions? Thanks!

Flags: needinfo?(agi)

What permissions are you looking to handle? for most of them now GeckoView uses Gecko's permission store so there shouldn't be anything special that you need to do for mobile specifically (just store the permission using nsIPermissionManager). Or maybe you need a higher level API than nsIPermissionManager?

Flags: needinfo?(agi)
Whiteboard: [webdriver:backlog]
Severity: normal → S3
Duplicate of this bug: 1804471
Product: Testing → Remote Protocol
Blocks: 1589554

:whimboo this bug blocks testing Screen Wake Lock, Bug 1589554. I can add a similar workaround using a pref to permissions.sys.mjs but what is preventing us from implementing this? I poked around a bit but my knowledge of marionette seems to not be enough to implement this.

Flags: needinfo?(hskupin)

We currently do not have the man power to get this feature implemented. As such it's in the backlog. So yes, you could update the code and do it similar as for other permissions if that works. We are happy to accept patches. Otherwise please let us know.

Flags: needinfo?(hskupin)
Summary: Implement Permissions API extension to WebDriver → Implement Permissions API extension for Marionette
Blocks: 1854996
See Also: 1854996 →
Priority: P3 → P2

We are going to implement this API for WebDriver BiDi first (bug 1875065). So it should then be easier to get it added for Marionette as well.

Depends on: 1875065
Assignee: nobody → krosylight
Status: NEW → ASSIGNED

Kagami, thank you for the patch but could you please move it to a different bug, which will block this one? There is more work to do to have a full implementation for Permissions in Marionette and that we want to do on this bug. Thanks!

Flags: needinfo?(krosylight)

I'll do, thanks!

Flags: needinfo?(krosylight)

Comment on attachment 9373310 [details]
Bug 1524074 - Generalize permission handling of storage-access to all permissions r=whimboo

Revision D198836 was moved to bug 1875257. Setting attachment 9373310 [details] to obsolete.

Attachment #9373310 - Attachment is obsolete: true
Assignee: krosylight → nobody
Status: ASSIGNED → NEW

On bug 1875724 the marionette.setpermission.enabled pref will be set for all web-platform tests. We should wait for that until we do the full implementation for Marionette.

Depends on: 1875724
Points: --- → 3
Whiteboard: [webdriver:backlog] → [webdriver:m11]
Assignee: nobody → aborovova
Status: NEW → ASSIGNED
Blocks: 1897180
Depends on: 1878741
No longer depends on: 1878741
Pushed by aborovova@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/041acf255585
[marionette] Add validation to "setPermission" API. r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/7d9f74554923
Enable "marionette.setpermission.enabled" preference for wpt tests. r=webdriver-reviewers,whimboo
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/46426 for changes under testing/web-platform/tests
Whiteboard: [webdriver:m11] → [webdriver:m11], [wptsync upstream]

https://hg.mozilla.org/mozilla-central/rev/041acf255585
https://hg.mozilla.org/mozilla-central/rev/7d9f74554923

Status: ASSIGNED → RESOLVED
Closed: 1 month ago
status-firefox128: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 128 Branch
Upstream PR was closed without merging
Duplicate of this bug: 1875724
Upstream PR was closed without merging
Upstream PR merged by moz-wptsync-bot
Pushed by wptsync@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d2fb2e755c8d
[wpt PR 46426] - [Gecko Bug 1524074] Enable "marionette.setpermission.enabled" preference for wpt tests., a=testonly
Whiteboard: [webdriver:m11], [wptsync upstream] → [webdriver:m11], [wptsync upstream][webdriver:relnote]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: