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

Convert Youtube to BasicController #64

Closed nemchik closed 9 years ago

nemchik commented 9 years ago

This converts the current non-basic controller to a basic controller and adds support for the experimental test tube player available here https://www.youtube.com/testtube This also adds support for thumbs up and down. This also makes efforts to detect the availability of the previous key function. In my testing detecting when it IS available is always successful, but detecting when it IS NOT available is not always successful. The detection failure is due to the way youtube uses html5 history rather than actually loading each page you visit, thus the controller is not reinitialized when you navigate from one video to the next because the page is not actually reloaded, but rather ajax and html5 change the content of the page dynamically while preserving the controller we're loading in the background. I was unable to find a way to handle detection of youtube's html5 history manipulation in order to call for controller re-initialization, but in the grand scheme the only negative affect is that the popup will sometimes display the previous key button when it is not available (but sometimes everything will work as expected!).

IMPORTANT This drops support for the flash player entirely. I have been unable to get the flash player working in various tests. One issue is that the player itself doesnt exist in the html code, but is rather replaced into the code using javascript, so detecting the player is inconsistent. Another issue is once the flash player is detected we have to either override the existing functions inside the controller (that are already being overridden to facilitate support for the current standard and test tube players) OR completely replace the entire controller. Replacing the controller with the former controller did not seem to work, likely because of the way the former controller was loaded vs the way basic controllers are loaded (thus the additional file changes in this PR aside from the controller file itself). There may still be a way to accomplish supporting the flash player inside the scope of a basic controller, but i'm stumped, and chrome doesnt use the flash player without forcing it via additional extensions which is a user choice.

msfeldstein commented 9 years ago

Cool I'll check this out when I'm back from Vacation

On Sun, Jun 14, 2015 at 9:08 AM Eric Nemchik notifications@github.com wrote:

This converts the current non-basic controller to a basic controller and adds support for the experimental test tube player available here https://www.youtube.com/testtube This also adds support for thumbs up and down. This also makes efforts to detect the availability of the previous key function. In my testing detecting when it IS available is always successful, but detecting when it IS NOT available is not always successful. The detection failure is due to the way youtube uses html5 history rather than actually loading each page you visit, thus the controller is not reinitialized when you navigate from one video to the next because the page is not actually reloaded, but rather ajax and html5 change the content of the page dynamically while preserving the controller we're loading in the background. I was unable to find a way to handle detection of youtube's html5 history manipulation in order to call for controller re-initialization, but in the grand scheme the only negative affect is that the popup will sometimes display the previous key button when it is not available (but sometimes everything will work as expected!).

IMPORTANT This drops support for the flash player entirely. I have been unable to get the flash player working in various tests. One issue is that the player itself doesnt exist in the html code, but is rather replaced into the code using javascript, so detecting the player is inconsistent. Another issue is once the flash player is detected we have to either override the existing functions inside the controller (that are already being overridden to facilitate support for the current standard and test tube players) OR completely replace the entire controller. Replacing the controller with the former controller did not seem to work, likely because of the way the former controller was loaded vs the way basic controllers are loaded (thus the additional file changes in this PR aside from the controller file itself). There may still be a way to accomplish supporting the flash player inside the scope of a basic controller, but i'm stumped, and chrome doesnt use the flash player without forcing it via additional extensions which is a user

choice.

You can view, comment on, or merge this pull request online at:

https://github.com/msfeldstein/chrome-media-keys/pull/64 Commit Summary

  • Convert Youtube to BasicController

File Changes

Patch Links:

— Reply to this email directly or view it on GitHub https://github.com/msfeldstein/chrome-media-keys/pull/64.

msfeldstein commented 9 years ago

Gonna try living with this for a bit then i'll push to the store. I'm totally fine dropping flash player support, no one in their right mind is using it at this point