kiwix / kiwix-apple

Kiwix for offline access on iOS and macOS
https://apple.kiwix.org
GNU Lesser General Public License v3.0
434 stars 69 forks source link

Video won't enter full screen on macOS #741

Closed BPerlakiH closed 3 days ago

BPerlakiH commented 3 weeks ago

On macOS, the video playback won't enter full screen. From the web inspector we have the following info, eg. taken from "Canadian Prepper" ZIM: video.min.js [Line 7311] error:

        enterFullScreen()
        {
            const e = this.el_;
            if (e.paused && e.networkState <= e.HAVE_METADATA)
                Xt(this.el_.play()),
                this.setTimeout(function() {
                    e.pause();
                    try {
                        e.webkitEnterFullScreen()
                    } catch (e) {
                        this.trigger("fullscreenerror", e)
                    }
                }, 0);
            else
                try {
                    e.webkitEnterFullScreen()     //    <===== Error: Unhandled Promise Rejection: InvalidStateError: The object is in an invalid state.
                } catch (e) {
                    this.trigger("fullscreenerror", e)
                }
        }

Possible work around: Replace this call or capture the invocation, and handle the "enter fullscreen" / "exit full screen" on the swift side by:

webView.window?.toggleFullScreen(sender:Any?)
benoit74 commented 2 weeks ago

I'm not sure to get it right, but I don't think you should fix this with a workaround on swift side. I mean, ZIMs should do their best to be compatible with all readers.

What would be the proper way to enter fullscreen with JS code that would work on apple reader? From the documentation, it looks like current code can only fail if the element is not allowed to enter fullscreen https://developer.apple.com/documentation/webkitjs/htmlvideoelement/1633500-webkitenterfullscreen

BPerlakiH commented 2 weeks ago

@benoit74 I am not sure to be honest. That property seems to be read only. I did found some other clues related, it might be that the meta-data is not yet loaded, when we try to put it to full screen. https://stackoverflow.com/a/20693480/4400317

BPerlakiH commented 2 weeks ago

@benoit74 I also do see some user-agent checking in the video.min.js file:

}, e.supportsFullScreen = function() {
                if ("function" == typeof this.el_.webkitEnterFullScreen) {
                    var e = y.navigator && y.navigator.userAgent || "";
                    if (/Android/.test(e) || !/Chrome|Mac OS X 10.5/.test(e))
                        return !0
                }
                return !1

I am not sure if it's relevant, but we might have different values than what's included there, so it might block the macOS fullscreen, because of this (just guessing now).

kelson42 commented 4 days ago

@BPerlakiH I believe by fixing this on macOS, you have broken the fullscreen feature on iOS. I get now (with latest tesflight) a black screen.

BPerlakiH commented 3 days ago

I've created a new PR to limit this change to macOS only: https://github.com/kiwix/kiwix-apple/pull/764