Allows disabling them for a specific origin in case it causes trouble or unwanted media controls.
BUG: 394126
Details
Details
- Reviewers
fvogt ognarb - Group Reviewers
Plasma VDG - Commits
- R856:ca085f500183: Add per-origin media controls blacklist
- Successfully enabled and disabled media controls on a domain
- Popup shows all domains of the current page (in case it has iframes)
- Shows only if there is a player in the current tab or user blacklisted an origin in that tab previously
- Opened popup on site without player, got dummy popup, started iframe player, got blacklist offered
There's a way for us to ship a blacklist by default that the user can then override
Should there be wildcard support? Do we need a UI in settings to list all blacklists and edit them?
When there are no controls whatsoever in the popup
Diff Detail
Diff Detail
- Repository
- R856 Plasma Browser Integration
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
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.
| extension/action_popup.js | ||
|---|---|---|
| 20 | getCurrentTabUrl() | |
| 42 | Currently the function name implies that it returns all tabs, so maybe rename to getCurrentTabFramesUrls() (or better)? | |
| 46 | Maybe runAt: "document_start" to speed it up a bit? I'm not sure about the implications. | |
| 62 | Whitespace? | |
| 118 | Currently calling set(domain, false); twice has a different result from set(domain, false); once, so maybe split into whitelist(domain) and blacklist(domain))? | |
| 120 | Whitespace? | |
| extension/content-script.js | ||
| 68–84 | Is this guaranteed to be identical to new URL(window.location.href).hostname as used in utils? | |
| 70 | Use mpris. instead of items.mpris | |
Comment Actions
AFAICT it doesn't reload the mpris state in the content-script immediately when settings change, can that be implemented?
First we need D24203.
It might be tricky to properly unload e.g. the media sessions stuff, but I can look into this.
Actually, I wanted to implement "live" settings changes for the other settings (like breeze scroll bars and what not) in a later step, too, for which D24203 is a prerequisite.
| extension/action_popup.js | ||
|---|---|---|
| 20 | The class is named TabUtils but I can change the method if you want.. | |
| 46 | Don't think this will change much, given it is only executed when you click the toolbar button, at which point the page is probably already loaded, but I'll give it a try | |
| 62 | Intentional, for a bit of visual grouping | |
| 118 | Imho the caller shouldn't have to care about whether it needs to be whitelisted or removed from the blacklist. The whitelist is only so that the user can opt-in to websites which we by default blacklist. | |
| extension/content-script.js | ||
| 68–84 | Good question, no idea, probably. :) | |
Comment Actions
- Show media controls only if there is a player or something blacklisted on that page, otherwise it gets annoying
| extension/action_popup.js | ||
|---|---|---|
| 217 | Does this also hit all the frames inside? | |
| extension/action_popup.js | ||
|---|---|---|
| 217 | Effectively, yes. It asks the extension.js whether it knows any players on the given tab, which will include all frames, see the actual implementation of "hasTabPlayer" which checks playerIds containing all of them. | |