D13717: Implement Seek() and Seeked()

2018-06-29 Thread Adam Mokhtari
mokhtari marked 3 inline comments as done. mokhtari added inline comments. INLINE COMMENTS > broulik wrote in content-script.js:177 > Please avoid negating a longer statement, this makes the code hard to > understand at a glance: > > if (activePlayer.seekable.length === 0) Good point.

D13717: Implement Seek() and Seeked()

2018-06-29 Thread Adam Mokhtari
mokhtari updated this revision to Diff 36902. mokhtari added a comment. Clean things up a bit: - use signal auto-relaying to clean up mprisplugin logic - use more readable test in content-script.js REPOSITORY R856 Plasma Browser Integration CHANGES SINCE LAST UPDATE

D13717: Implement Seek() and Seeked()

2018-06-29 Thread Kai Uwe Broulik
broulik added inline comments. INLINE COMMENTS > content-script.js:177 > +if (activePlayer) { > +if (!(activePlayer.seekable.length > 0)) { > +console.warn("Got seek command, but player is not seekable"); Please avoid negating a longer statement, this makes the code hard

D13717: Implement Seek() and Seeked()

2018-06-28 Thread Adam Mokhtari
mokhtari added a comment. Regarding loops with the Media Player widget, I haven't noticed any (using KDE Neon, 5.13.1). What test would catch this? INLINE COMMENTS > broulik wrote in content-script.js:177 > The `seekable` property isn't an int but a `TimeRanges` object that tells you >

D13717: Implement Seek() and Seeked()

2018-06-28 Thread Adam Mokhtari
mokhtari updated this revision to Diff 36854. mokhtari added a comment. Fixed seekable check. REPOSITORY R856 Plasma Browser Integration CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13717?vs=36634=36854 REVISION DETAIL https://phabricator.kde.org/D13717 AFFECTED FILES

D13717: Implement Seek() and Seeked()

2018-06-27 Thread Kai Uwe Broulik
broulik added a comment. Thanks for your patch! There's a minor issue but other than that it's good. Can you also verify this doesn't cause a loop when using Plasma's media controller applet? INLINE COMMENTS > content-script.js:177 > +if (activePlayer) { > +if

D13717: Implement Seek() and Seeked()

2018-06-25 Thread Nicolas Fella
nicolasfella added a comment. +1, setting the position for e.g. Youtube now works from KDE Connect REPOSITORY R856 Plasma Browser Integration REVISION DETAIL https://phabricator.kde.org/D13717 To: mokhtari, broulik Cc: nicolasfella, plasma-devel, #kde_connect, jdvr, yannux, Danial0_0,

D13717: Implement Seek() and Seeked()

2018-06-25 Thread Adam Mokhtari
mokhtari created this revision. mokhtari added a reviewer: KDE Connect. mokhtari added a project: KDE Connect. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. mokhtari requested review of this revision. REVISION SUMMARY Implementing