D24194: Add per-domain media controls blacklist

[Fabian Vogt]

fvogt added a comment.


  AFAICT it doesn't reload the mpris state in the content-script immediately when settings change, can that be implemented?
  
  IMO the blacklist should be more than the domain, it should test the same conditions as CORS, so protocol, domain and port.

INLINE COMMENTS

> action_popup.js:20
> +    // Gets the URL of the currently viewed tab
> +    static getCurrentUrl() {
> +        return new Promise((resolve, reject) => {

`getCurrentTabUrl()`

> action_popup.js:42
> +    // Gets the URLs of the currently viewed tab including all of its iframes
> +    static getCurrentUrls() {
> +        return new Promise((resolve, reject) => {

Currently the function name implies that it returns all tabs, so maybe rename to `getCurrentTabFramesUrls()` (or better)?

> action_popup.js:46
> +                allFrames: true, // so we also catch iframe videos
> +                code: `window.location.href`
> +            }, (result) => {

Maybe `runAt: "document_start"` to speed it up a bit? I'm not sure about the implications.

> action_popup.js:62
> +        return new Promise((resolve, reject) => {
> +
> +            Promise.all([

Whitespace?

> action_popup.js:118
> +
> +    set(domain, block) {
> +        return this.get().then((blockInfo) => {

Currently calling `set(domain, false);` twice has a different result from `set(domain, false);` once, so maybe split into `whitelist(domain)` and `blacklist(domain)`)?

> action_popup.js:120
> +        return this.get().then((blockInfo) => {
> +
> +            let whitelist = blockInfo.mprisSettings.whitelistedDomains;

Whitespace?

> content-script.js:68
>      if (mpris.enabled) {
> -        loadMpris();
> -        if (items.mprisMediaSessions.enabled) {
> -            loadMediaSessionsShim();
> +        const domain = window.location.hostname;
> +

Is this guaranteed to be identical to `new URL(window.location.href).hostname` as used in utils?

> content-script.js:70
> +
> +        const whitelist = items.mpris.whitelistedDomains || [];
> +        const blacklist = items.mpris.blacklistedDomains || [];

Use `mpris.` instead of `items.mpris`

REPOSITORY
  R856 Plasma Browser Integration

REVISION DETAIL
  https://phabricator.kde.org/D24194

To: broulik, #plasma, #vdg, fvogt, ognarb
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190926/d5ad555c/attachment.html>