search

msfeldstein / chrome-media-keys

Chrome extension that adds media keys to most web music players
GNU Affero General Public License v3.0
89 stars 33 forks source link

BasicController with Frames #55

Closed nemchik closed 9 years ago

nemchik commented 9 years ago

Currently the basic controller doesnt work if the player is in a frame like spotify. I've tried using overrides but it makes the controller pretty complex and kind of defeats the purpose. Can we add a property for frameSelector ? If frameSelector is defined in the controller then use something like this document.querySelector(frame).contentDocument.querySelector(element) whenever another selector is trying to get what it needs.

This is a bit of a bigger change so i'll see what I can do about a PR but i'm not really sure about the idea yet.

msfeldstein commented 9 years ago

Accessing stuff in frames has always been tricky but I think as long as the frames are served from exactly the same tld it might work. Experimenting in the developer console would probably answer whether this is a good idea or not.

The alternative is to just inject the controller in every frame and just have it do nothing if its in a frame with no player.

nemchik commented 9 years ago

Same TLD is the only thing that works anymore. Chrome (and most browsers) blocks cross site stuff. The spotify controller only works because of the frame code, so my goal with bringing this up is to try and simplify hooking into frames with the basic controller.

On Wed, May 20, 2015, 11:59 AM Michael Feldstein notifications@github.com wrote:

Accessing stuff in frames has always been tricky but I think as long as the frames are served from exactly the same tld it might work. Experimenting in the developer console would probably answer whether this is a good idea or not.

The alternative is to just inject the controller in every frame and just have it do nothing if its in a frame with no player.

— Reply to this email directly or view it on GitHub https://github.com/msfeldstein/chrome-media-keys/issues/55#issuecomment-103959635 .

nemchik commented 9 years ago

Here is what I have so far. https://github.com/nemchik/chrome-media-keys/commit/5c5ef67882104182f3c6404019b6eaafb9f1e4a9 I haven't made a pull yet because of https://github.com/nemchik/chrome-media-keys/blob/5c5ef67882104182f3c6404019b6eaafb9f1e4a9/src/controllers/BasicController.js#L164-L166, i don't really know how to handle those because I have never seen !! before so i don't know what it does, haha.

nemchik commented 9 years ago

Ok, maybe https://github.com/msfeldstein/chrome-media-keys/pull/56 is the right way to go.

nemchik commented 9 years ago

56 does finally solve this issue.