Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open Jellyfin in iframe to fix scrolling in webOS 3 #14

Merged
merged 6 commits into from
Feb 10, 2020

Conversation

dmitrylyzo
Copy link
Contributor

@dmitrylyzo dmitrylyzo commented Jan 19, 2020

This PR requires "Go back" handling in jellyfin-web and have no backward compatibility.
Perhaps backward compatibility can be added by checking the version less than 10.5.0 and performing history.back on frame contentWindow.

Opening Jellyfin in iframe fixes scrolling issue in webOS 3. But then handling of "Back" button in iframe is broken. So, we have to handle it manually (almost finished).

Also, in this PR I pass webOSNativeShell object into jellyfin-web to be able to "exit" in platform-specific way.

Inject CSS to hide scrollbar.

Some flaws
A vertical scrollbar appears. Adding scrolling="no" to iframe makes it unscrollable by wheel (but scrollable by ScrollManager). A possible solution (if scrollbar should be hidden) is to add in jellyfin-web something like:

.layout-tv body::-webkit-scrollbar {
    display: none;
}

But non-app TV-layout users will be affected too.

Important note
After loading JF (in the frame), you cannot return to the server form (for example, to change server) without restarting the application.

@dmitrylyzo
Copy link
Contributor Author

The trick with passing object into iframe didn't work for Tizen - object defined, but its content is not. CORS I think.
So, this works in webOS 3/4 emulator, but who knows how it will be on real TV or webOS 5. In worst case, adapter injecting and postMessage will be required - this works for Tizen.

@EraYaN
Copy link
Member

EraYaN commented Jan 20, 2020

I wonder if this would be allowed by LG, but we'll cross that bridge when we get there.

@JustAMan
Copy link
Contributor

A possible solution (if scrollbar should be hidden) is to add in jellyfin-web something like:

I think we can add this override in jf-web by inspecting if injected webOS object is defined somewhere, is that possible?

@dmitrylyzo
Copy link
Contributor Author

I think we can add this override in jf-web by inspecting if injected webOS object is defined somewhere, is that possible?

I tried to load webos.css (new file with mentioned style), if there is webOS. But load event fired after onAppReady (site.js) - too late. Sometimes it works, but it is not reliable.
In Tizen it is always late.

I did not find the "right" way to capture DOMContentLoaded of frame document. I have something that works, but it looks weird.

"ASAP" inject
    var contentFrame = document.querySelector('#contentFrame');
    var contentWindow = contentFrame.contentWindow;

    var timer;
    var loaded = false;

    function onLoad() {
        if (loaded) {
            return;
        }

        loaded = true;

        clearInterval(timer);
        contentFrame.contentDocument.removeEventListener('DOMContentLoaded', onLoad);

        console.log('WebOS adapter');

        contentWindow.webOS = webOS;
    }

    var domAttached = false;

    contentWindow.addEventListener('unload', function() {
        timer = setInterval(function () {
            switch (contentFrame.contentDocument.readyState) {
                case 'loading':
                    if (!domAttached) {
                        contentFrame.contentDocument.addEventListener('DOMContentLoaded', onLoad);
                        domAttached = true;
                    } else { // Securely connected to DOMContentLoaded - can stop timer
                        clearInterval(timer);
                    }
                    break;
    
                // In the case of "loading" is not caught
                case 'interactive':
                    onLoad();
                    break;
            }
        }, 0);
    });

    // In the case of "loading" and "interactive" are not caught
    contentFrame.addEventListener('load', onLoad);

@JustAMan
Copy link
Contributor

Yeah, doing it in setInterval() was my idea as well. This is certainly a workaround, but nothing too bad IMHO. So we should be able to use JS to add this CSS rule override to scrolling area if we detect presence of webos object, IMHO.

@dmitrylyzo
Copy link
Contributor Author

a32e355 (simplified this a bit)

Important note (also added in the first message)
After loading JF (in the frame), you cannot return to the server form (for example, to change server) without restarting the application.

@JustAMan
Copy link
Contributor

you cannot return to the server form (for example, to change server) without restarting the application

Do you have any idea why? unrelated note: why don't you join us on matrix?

@dmitrylyzo
Copy link
Contributor Author

Do you have any idea why?

Because I "exit app" (webOS.platformBack in jellyfin-web) when no "go back" remains (we are on home page - appRoute.canGoBack).

@dmitrylyzo
Copy link
Contributor Author

Navigation now (if the corresponding PRs are merged)
Returning from the login page (transparent) requires some extra changes in jf-web.
jellyfin-page-nav1

I tried to implement a minimal NativeShell - almost exact copy of "expo".

  • Where to get AppInfo.deviceId?
  • Supported features are questionable - I just took ones that jf-web reported for emulator.
  • DeviceProfile - as in "expo"
  • SyncProfile - as in "expo"
  • Since webOS object will be gone, need to look at something else to insert CSS. (NativeShell + browser.web0s)?

If we go NativeShell-way, then PR with "Go back" can be simplified a bit - drop webOS related lines.

Navigation with webOS-wrapper or NativeShell
In fact, as it was before (or so).
webOS-wrapper is an injected object with platformBack function that sends message to the top window.
Returning from the login page (transparent) requires some extra changes in jf-web.
jellyfin-page-nav2

@JustAMan
Copy link
Contributor

  • Since webOS object will be gone, need to look at something else to insert CSS. (NativeShell + browser.web0s)?

We can ship our own copy of webOS.js which defines webOS object within jf-web, and enable it either by detecting a user agent or when explicitly asked by appending some special suffix to the URL

Copy link
Contributor

@JustAMan JustAMan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully you tested that :)

function injectScript(document, url) {
var xhr = new XMLHttpRequest();
xhr.open('GET', url);
xhr.onload = function () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we at least alert() a user if xhr failed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alert is not displayed. I can probably try to preload the injected script before actually connecting to the server, and then displayError in case of xhr failure.

@dmitrylyzo
Copy link
Contributor Author

@JustAMan This PR can create problems for those who have frame-restrictive proxies - jellyfin/jellyfin-web#675 (comment)
I already thought of making a Tizen app on a similar principle (for myself, I call it a "launcher"). And now I doubt it.

@dkanada
Copy link
Member

dkanada commented Jan 27, 2020

@dmitrylyzo why do you doubt it?

@dmitrylyzo
Copy link
Contributor Author

@dmitrylyzo why do you doubt it?

Someone could just deny frames, and then app won't work.

@JustAMan
Copy link
Contributor

That would be their problem, no? :) Jumping through lots of extra hoops feels too much here.
I imagine there would be rather low amount of users who would use a TV connecting to a remote JF server over public Internet, and making a reverse proxy configuration which denies iframes publicly but allows them on your LAN is relatively easy.

@dmitrylyzo
Copy link
Contributor Author

Tried to add backward compatibility.
Version 10.4.3 does not have TV-friendly features added in master. So, this is only to restore history navigation.
Both methods work, but with some remarks.

in webOS adapter - won't work with `NativeShell` (when we create it) because it will return its version instead of `jellyfin-web`. Or just need to add an extra function (to retrieve `jellyfin-web` version) in `appHost`.
diff --git a/org.jellyfin.webos/js/webOS.js b/org.jellyfin.webos/js/webOS.js
index 8fde4e7..fa19178 100644
--- a/org.jellyfin.webos/js/webOS.js
+++ b/org.jellyfin.webos/js/webOS.js
@@ -3,6 +3,43 @@
 
     console.log('WebOS adapter');
 
+    /**
+     * Compares two versions.
+     * X.Y is less than X.Y.0
+     *
+     * @param {string} v1 version 1
+     * @param {string} v2 version 2
+     * @returns {number} -1 v1 < v2
+     * @returns {number} 0 v1 = v2
+     * @returns {number} 1 v1 > v2
+     */
+    function versionCompare(v1, v2) {
+        if (typeof v1 !== 'string' || typeof v2 !== 'string') {
+            throw new TypeError('Versions must be strings');
+        }
+
+        var arr1 = v1.split('.');
+        var arr2 = v2.split('.');
+
+        for (var i = 0, n = Math.max(arr1.length, arr2.length); i < n; i++) {
+            if (i >= arr1.length && i < arr2.length) {
+                return -1;
+            } else if (i < arr1.length && i >= arr2.length) {
+                return 1;
+            } else {
+                var num1 = parseInt(arr1[i]);
+                var num2 = parseInt(arr2[i]);
+                if (num1 < num2) {
+                    return -1;
+                } else if (num1 > num2) {
+                    return 1;
+                }
+            }
+        }
+
+        return 0;
+    }
+
     function postMessage(type, data) {
         window.top.postMessage({
             type: type,
@@ -15,4 +52,52 @@
             postMessage('AppHost.exit');
         }
     };
+
+    window.addEventListener('load', function () {
+        require(['apphost'], function(appHost) {
+            console.log('Version: ' + appHost.appVersion());
+
+            // FIXME: Need a jellyfin-web version - won't work with NativeShell
+            if (versionCompare(appHost.appVersion(), '10.5') === -1) {
+                console.log("Turn on 'Back' button handling");
+
+                var historyDepth = 1;
+
+                document.addEventListener('keydown', function (e) {
+                    switch (e.keyCode) {
+                        // Back
+                        case 461:
+                            if (historyDepth > 1) {
+                                history.back();
+                            } else {
+                                webOS.platformBack();
+                            }
+                            break;
+                    }
+                });
+
+                window.addEventListener('pushState', function(e) {
+                    historyDepth++;
+                });
+
+                window.addEventListener('popstate', function() {
+                    historyDepth--;
+                });
+
+                // Add 'pushState' and 'replaceState' events
+                var _wr = function(type) {
+                    var orig = history[type];
+                    return function() {
+                        var rv = orig.apply(this, arguments);
+                        var e = new Event(type);
+                        e.arguments = arguments;
+                        window.dispatchEvent(e);
+                        return rv;
+                    };
+                };
+                history.pushState = _wr('pushState');
+                history.replaceState = _wr('replaceState');
+            }
+        });
+    });
 })();

or

in separate module - no script caching (can be added); actually checks server version instead of `jellyfin-web`
--- /dev/null   2020-01-28 08:44:40.831999968 +0300
+++ b/org.jellyfin.webos/js/backbutton.js       2020-01-28 20:35:26.000000000 +0300
@@ -0,0 +1,47 @@
+(function() {
+    'use strict';
+
+    console.log("Turn on 'Back' button handling");
+
+    window.addEventListener('load', function () {
+        var historyDepth = 1;
+
+        document.addEventListener('keydown', function (e) {
+            switch (e.keyCode) {
+                // Back
+                case 461:
+                    if (historyDepth > 1) {
+                        history.back();
+                    } else {
+                        webOS.platformBack();
+                    }
+                    break;
+            }
+        });
+
+        window.addEventListener('pushState', function(e) {
+            historyDepth++;
+        });
+
+        window.addEventListener('popstate', function() {
+            historyDepth--;
+        });
+
+        // Add 'pushState' and 'replaceState' events
+        var _wr = function(type) {
+            var orig = history[type];
+            return function() {
+                var rv = orig.apply(this, arguments);
+                var e = new Event(type);
+                e.arguments = arguments;
+                window.dispatchEvent(e);
+                return rv;
+            };
+        };
+        history.pushState = _wr('pushState');
+        history.replaceState = _wr('replaceState');
+    });
+})();
diff --git a/org.jellyfin.webos/js/index.js b/org.jellyfin.webos/js/index.js
index 05c3168..7b5e707 100644
--- a/org.jellyfin.webos/js/index.js
+++ b/org.jellyfin.webos/js/index.js
@@ -1,7 +1,6 @@
 var curr_req = false;
 var server_info = false;
 var manifest = false;
-var textToInject = false;
 
 //Adds .includes to string to do substring matching
 if (!String.prototype.includes) {
@@ -16,6 +15,43 @@ if (!String.prototype.includes) {
   };
 }
 
+/**
+ * Compares two versions.
+ * X.Y is less than X.Y.0
+ *
+ * @param {string} v1 version 1
+ * @param {string} v2 version 2
+ * @returns {number} -1 v1 < v2
+ * @returns {number} 0 v1 = v2
+ * @returns {number} 1 v1 > v2
+ */
+function versionCompare(v1, v2) {
+    if (typeof v1 !== 'string' || typeof v2 !== 'string') {
+        throw new TypeError('Versions must be strings');
+    }
+
+    var arr1 = v1.split('.');
+    var arr2 = v2.split('.');
+
+    for (var i = 0, n = Math.max(arr1.length, arr2.length); i < n; i++) {
+        if (i >= arr1.length && i < arr2.length) {
+            return -1;
+        } else if (i < arr1.length && i >= arr2.length) {
+            return 1;
+        } else {
+            var num1 = parseInt(arr1[i]);
+            var num2 = parseInt(arr2[i]);
+            if (num1 < num2) {
+                return -1;
+            } else if (num1 > num2) {
+                return 1;
+            }
+        }
+    }
+
+    return 0;
+}
+
 
 function isVisible(element) {
     return element.offsetWidth > 0 && element.offsetHeight > 0;
@@ -218,7 +254,7 @@ function handleSuccessServerInfo(data, baseurl, auto_connect) {
         }
     }
 
-    storage.set('connected_server', { 'baseurl': baseurl, 'auto_connect': auto_connect, 'id': data.Id })
+    storage.set('connected_server', { 'baseurl': baseurl, 'auto_connect': auto_connect, 'id': data.Id, 'version': data.Version })
 
     getManifest(baseurl)
 }
@@ -238,8 +274,8 @@ function handleSuccessManifest(data, baseurl) {
     storage.set('connected_server', info)
     console.log(info);
 
-    getTextToInject().then(function () {
-        handoff(hosturl);
+    getTextToInject(info.version).then(function (textToInject) {
+        handoff(hosturl, textToInject);
     }).catch(function (error) {
         console.error(error);
         displayError(error);
@@ -281,29 +317,45 @@ function abort() {
     console.log("Aborting...");
 }
 
-function getTextToInject() {
-    var url = 'js/webOS.js';
-
+function loadUrl(url) {
     return new Promise(function (resolve, reject) {
-        if (textToInject) {
-            resolve(textToInject);
-        } else {
-            var xhr = new XMLHttpRequest();
+        var xhr = new XMLHttpRequest();
 
-            xhr.open('GET', url);
+        xhr.open('GET', url);
 
-            xhr.onload = function () {
-                textToInject = xhr.responseText;
-                resolve(textToInject);
-            };
+        xhr.onload = function () {
+            resolve(xhr.responseText);
+        };
 
-            xhr.onerror = function () {
-                reject("Failed to load '" + url + "'");
-            }
-
-            xhr.send();
+        xhr.onerror = function () {
+            reject("Failed to load '" + url + "'");
         }
+
+        xhr.send();
+    });
+}
+
+function getTextToInject(version) {
+    var textToInject = '';
+
+    var urls = ['js/webOS.js'];
+
+    // Add 'Back' button handling for older versions
+    if (versionCompare(version, '10.5') === -1) {
+        urls.push('js/backbutton.js');
+    }
+
+    var p = Promise.resolve();
+
+    urls.forEach(function (url) {
+        p = p.then(function () {
+            return loadUrl(url);
+        }).then(function (data) {
+            return textToInject += data;
+        });
     });
+
+    return p;
 }
 
 function injectScriptText(document, text) {
@@ -313,7 +365,7 @@ function injectScriptText(document, text) {
     document.head.appendChild(script);
 }
 
-function handoff(url) {
+function handoff(url, textToInject) {
     console.log("Handoff called with: ", url)
     //hideConnecting();
 

@heyhippari
Copy link

I think getting back to the WebOS menu from the home screen is a nicer (and more expected) experience than getting back to the server input form.

Other apps act that way, as well, and people are more likely to want to get back to the home menu than to change server often.

Imo, adding a "Change server" entry to the user menu on WebOS that would bring the user back to the initial form is a better solution for that use case.

@dmitrylyzo
Copy link
Contributor Author

webos-flow1

In jellyfin-web there is multiserver feature - show "Change Server" button on login page and "Select Server" button in profile settings. It opens selectserver page - this works for app with bundled web (Tizen, Android). This also works in a webOS app, but we will never return to the first form.

We can add some feature (or other way) to indicate that selectserver should be performed by app and NativeShell.AppHost.changeServer function to open native selectserver page. Then I should implement NativeShell (its minimal form is complete).

@JustAMan
Copy link
Contributor

I'm fine with any way of adding ability to choose a server, but I think showing app's page is slightly better because it allows user to change what they set and then set a "remember this address" checkbox.

@dmitrylyzo
Copy link
Contributor Author

Switched to NativeShell.
Supported features were taken from what was previously reported. displaymode is excluded - we are on TV.
webos-flow2
Same as above, only a little more detailed.

Requires PR jellyfin/jellyfin-web#755

In my opinion, only a question of support for Jellyfin 10.4.x and below is remained.

@EraYaN
Copy link
Member

EraYaN commented Feb 3, 2020

I think we can safely assume that no it will ship with 10.5 and 10.4 is not supported. We can even add a check in the current manifest download on the webOS side.

@dmitrylyzo
Copy link
Contributor Author

dmitrylyzo commented Feb 4, 2020

I think we can safely assume that no it will ship with 10.5 and 10.4 is not supported.

I tried to add backward compatibility to the app here, but this is to much for single version which will be replaced soon (I hope). If support for 10.4 will be required, it is easier to add "Back" handling to web instead.

For now, I think with PR jellyfin/jellyfin-web#755 it is ready for use with "nightly".

Just to let you know
While I was trying to solve keyboardnavigation+input conflict, I discovered some system events:

  • keyboardStateChange
  • webOSMouse

These events aren't sent to the frame. They are not in use.

@dmitrylyzo dmitrylyzo marked this pull request as ready for review February 4, 2020 19:31
Copy link
Member

@EraYaN EraYaN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you test this on webOS 4.1? (2018 OLEDs) Otherwise I'll give it a go tonight. Code looks good.

@dmitrylyzo
Copy link
Contributor Author

Did you test this on webOS 4.1? (2018 OLEDs) Otherwise I'll give it a go tonight. Code looks good.

I have only emulators. I tested 3.0 and 4.0

@EraYaN
Copy link
Member

EraYaN commented Feb 5, 2020

Alright I'll do it tonight (GMT+1).

@EraYaN
Copy link
Member

EraYaN commented Feb 5, 2020

Alright so of course part of the CacheStorage appStorage.js stuff is broken on non https endpoints. But the app does not seem to care that much. There is a (very ugly) scrollbar sadly. There is still some focus element wonkyness but it's much better already.
The detail page does not display correctly though.
(With is the 20200205 nightly). Most of these will be fixed in web I believe, but the icons are still a mess.
webos-example

@dmitrylyzo
Copy link
Contributor Author

@EraYaN EraYaN merged commit 3db27a8 into jellyfin:master Feb 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants