mozilla-mobile / fenix

⚠️ Fenix (Firefox for Android) moved to a new repository. It is now developed and maintained as part of: https://github.com/mozilla-mobile/firefox-android
https://github.com/mozilla-mobile/firefox-android
Mozilla Public License 2.0
6.47k stars 1.28k forks source link

Videos with different aspect ratio than device are zoomed in #8252

Closed Teelry closed 1 year ago

Teelry commented 4 years ago

Videos with a smaller aspect ratio will be zoomed in. For example, videos in 16:9 running on a device that has an 18:9 aspect ratio will zoom them in, making the user able to scroll up/down the video, which can be problematic if the user is watching a video with subtitles. It should be either not zoomed in, or given the choice to.

I realise this is low priority, but it is still a problem.

Status as of Feb-2022: YouTube issue is not a Firefox issue. They are doing user agent detection that only works in Chrome. A Chrome engineer has opened a YouTube bug with them. It is possible to work around this using Ublock

┆Issue is synchronized with this Jira Task

jxu commented 3 years ago

it didn't work for me when I tried it out a few months ago now if I have a long video, I just download it with youtube-dl lol

emilio commented 3 years ago

This is the kind of bug that I didn't know about for a long time, and now I found it and I can't unsee, so if nobody gets to it sooner I try to poke at fixing it :)

jxu commented 3 years ago

also what I do is just put the YouTube webpage in landscape mode instead of pressing fullscreen. then if you scroll correctly the video is almost the same as proper fullscreen :p

On Mon, Feb 22, 2021, 7:58 PM Emilio Cobos Álvarez notifications@github.com wrote:

This is the kind of bug that I didn't know about for a long time, and now I found it and I can't unsee, so if nobody gets to it sooner I try to poke at fixing it :)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mozilla-mobile/fenix/issues/8252#issuecomment-783789149, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB46VXTUEWWZQMLVTCQASLDTAL4RTANCNFSM4KR64JVQ .

emilio commented 3 years ago

This seems youtube specific. The fullscreen element is the right size, but youtube somehow ends up setting the <video> element height in a way that's taller than the viewport. Not sure yet where they get that size from.

intrnl commented 3 years ago

Also worth noting that while Chrome does proper fullscreen for YouTube videos, it doesn't actually show captions properly, it ends up being shifted way below the viewport. I hope that in the chance that widescreen YT videos on Firefox gets fixed, this other issue also gets taken care of :smile:

emilio commented 3 years ago

This might just be a youtube bug, fwiw. I'll dig a bit more before reaching out to them though.

emilio commented 3 years ago

So some observations. This all comes from getPlayerSize() in the youtube code. That function does something sorta like this:

    g.k.getPlayerSize = function() {
        var a = this.app.V()
          , b = this.app.ma.isFullscreen();
        if (b && Ll())
            return new g.cf(window.outerWidth,window.outerHeight);
        if (b || a.bf) {
            if (window.matchMedia) {
                a = "(width: " + window.innerWidth + "px) and (height: " + window.innerHeight + "px)";
                this.em && this.em.media === a || (this.em = window.matchMedia(a));
                var c = this.em && this.em.matches
            }
            if (c)
                return new g.cf(window.innerWidth,window.innerHeight)
        } else {
            if (!isNaN(this.Gl.width) && !isNaN(this.Gl.height))
                return this.Gl.clone();
            if (this.Oo && this.Ks && !this.app.isWidescreen())
                for (a = 0; a < this.Oo.length; a++)
                    if (b = this.Oo[a],
                    b.query.matches)
                        return new g.cf(b.size.width,b.size.height)
        }
        return new g.cf(this.element.clientWidth,this.element.clientHeight)
    }

Where Ll() is defined as: return Kl("android") && Kl("chrome") && !(Kl("trident/") || Kl("edge/")) && !Kl("cobalt"), and Kl just tests 0 <= navigator.userAgent.toLowerCase().indexOf(a).

I'd note that Chrome does get an identically wrong size as Firefox at one point, but then it gets another resize with the "good" size.

So on chrome, they use a different codepath which uses outerHeight. If they did that on Firefox they should get a reasonable size, I'd think. I found a quirk where outerHeight was slightly smaller than innerHeight: https://bugzilla.mozilla.org/show_bug.cgi?id=712225.

But ok, they don't take the outerHeight codepath, so we go through the innerWidth + matchMedia codepath. But my phone has a fractional devicePixelRatio (2.608695652173913). So that won't work, we end up with (width: 846px) and (height: 414px), but the real dimensions are fractional and this is width: 846.0166625976562, height: 414. This explains why for some people changing display scaling fixes it. Note that we tried to make innerWidth / innerHeight not round a while ago (https://github.com/w3c/csswg-drafts/issues/5260), but that didn't work and broke tons of websites.

So we end up falling back to this.element.clientWidth and this.element.clientHeight codepath, which is the resize that ends up being the same as chrome (but then chrome gets fixed in the subsequent resize because of the UA sniffing taking the fullscreen codepath). This would be fixed if in fullscreen they just looked at fullscreenElement.clientWidth/clientHeight for example.

So I think this mostly points to a youtube-side bug. I'll look into contacting them, and I'll look into the outer{Width,Height} bug separately.

emilio commented 3 years ago

I also verified that the innerWidth / innerHeight codepath would not be taken in chrome on my device, for the same reason.

cmicek1 commented 3 years ago

It's not just YouTube, at least for me. Some sites that use jwplayer have the issue too (though dragging the video up fixes it right away): https://www.jwplayer.com/developers/web-player-demos/adaptive-streaming/

There are some others that I've come across as well, but this was just an easy counterexample for me to find right away

N3tFX commented 3 years ago

Also for me too, it's not only on YouTube. Plus, if it was a bug in the YouTube code, shouldn't also the desktop version of Firefox have the same problem? Does the desktop version of Firefox handles the code differently?

emilio commented 3 years ago

It's not just YouTube, at least for me. Some sites that use jwplayer have the issue too (though dragging the video up fixes it right away): https://www.jwplayer.com/developers/web-player-demos/adaptive-streaming/

So, I couldn't repro that issue on my device (on Firefox Nightly), so perhaps there are in fact multiple issues lurking around... That video shows up with the correct aspect ratio for me (it's 846.017×414, which matches my screen height).

Are you using Firefox Nightly? Or release? Anything special of your device? Weird DPR / screen size / etc?

Also for me too, it's not only on YouTube. Plus, if it was a bug in the YouTube code, shouldn't also the desktop version of Firefox have the same problem? Does the desktop version of Firefox handles the code differently?

In the desktop version of Firefox the video is not (usually at least, I guess) taller than your screen, so no. Plus I suspect the desktop and mobile versions of YouTube serve different code anyhow. If you send me other links where I could possibly reproduce similar issues I can look, but so far the issue I can reproduce is the YouTube one, and that I investigated (to the best of my knowledge) above.

FWIW the easiest way to investigate these is just via about:debugging. Inspecting the tab, finding the relevant <video> element in the inspector, and then figuring out why it has the wrong size.

emilio commented 3 years ago

So, yeah, there are multiple bugs here at play, I think... @agi pointed me to https://vimeo.com/76979871, where I can repro the issue on my device (Fenix screenshot). However! I can also repro it on Chrome to a lesser extent.

But his devices didn't repro the Youtube issue I described above... So there are likely multiple issues at play here, some of them being website issues (maybe not dealing with non-integer scale factors like the YouTube issue above?), maybe some others being Gecko or Blink issues or both.

I'm moderately sure the YouTube issue described above is likely a YouTube bug. The Vimeo issue seems also potentially a site issue, but I can debug it further to see if what they're doing looks reasonable now that I have a repro :)

emilio commented 3 years ago

The vimeo issue off hand also seems like a CSS issue on their side. Their player has:

.player:fullscreen {
    width: 100% !important;
    height: 100% !important;
}

that should do the right thing, but they also have:

.player {
    min-height: 180px;
    padding-bottom: 56.25% !important;
}

(And box-sizing: border-box in another rule)

All browsers agree that the padding wins in this case, test-case:

<!doctype html>
<style>
body { margin: 0 }
div {
  box-sizing: border-box;
  width: 100%;
  height: 100%;
  padding-bottom: 100%;
  background: red;
}
</style>
<div></div>

So if the aspect ratio of your screen is greater than 16/9 (that's the 56.25%), then the player is going to overflow, and sadness ensues.

emilio commented 3 years ago

Contacting Vimeo as well about this. So the Vimeo issue is exclusive of phones with aspect ratio larger than 16/9, while the youtube issue could be more general, depending on the actual aspect ratio of the video and DPR of the phone, IIUC.

emilio commented 3 years ago

So the difference between Firefox and Chrome in Vimeo is that Chrome doesn't use the area of the android buttons etc for Fullscreen, while Firefox does, so Chrome triggers the issue on my phone, but less severely.

emilio commented 3 years ago

Ok, so the youtube issue doesn't repro on @agi's devices because those have a devicePixelRatio of two (so they match the media-query codepath in my comment above). The Vimeo issue happens on his larger device, so it all checks out so far.

Nandru86 commented 3 years ago

This might just be a youtube bug, fwiw. I'll dig a bit more before reaching out to them though.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mozilla-mobile/fenix/issues/8252#issuecomment-784253236, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFP7B2L3K7WVBHWTEO74W7LTAO5VPANCNFSM4KR64JVQ .

I can confirm it affects other video sites besides youtube. Both SFW and NSFW. And using different kinds of video players

jxu commented 3 years ago

it happens to me on vimeo and other sites like panopto and zoom

On Tue, Feb 23, 2021 at 9:06 PM emmandyar notifications@github.com wrote:

This might just be a youtube bug, fwiw. I'll dig a bit more before reaching out to them though.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub < https://github.com/mozilla-mobile/fenix/issues/8252#issuecomment-784253236 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AFP7B2L3K7WVBHWTEO74W7LTAO5VPANCNFSM4KR64JVQ

.

I can confirm it affects other video sites besides youtube. Both SFW and NSFW. And using different kinds of video players

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mozilla-mobile/fenix/issues/8252#issuecomment-784691295, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB46VXRRFASDPRIEXCK3MELTARNIRANCNFSM4KR64JVQ .

emilio commented 3 years ago

Send links to pages that you can repro in? Otherwise stuff won't magically get fixed. I tried air.mozilla.org (a Panopto thingie) and the Zoom recording linked from here, and I couldn't reproduce the issue, the videos weren't cut off.

linsui commented 3 years ago

https://www.xinpianchang.com/a11093700 Full screen is broken in desktop mode.

jxu commented 3 years ago

I am using a Google pixel 4a, which has a holepunch selfie camera and a 19.5:9 (what should be called 13:6 hmm) aspect ratio. If you have an Android phone then you can try with those settings.

On Wed, Feb 24, 2021, 5:04 AM Emilio Cobos Álvarez notifications@github.com wrote:

Send links to pages that you can repro in? Otherwise stuff won't magically get fixed. I tried air.mozilla.org (a Panopto thingie) and the Zoom recording linked from here https://mail.mozilla.org/pipermail/firefox-dev/2021-February/007650.html, and I couldn't reproduce the issue, the videos weren't cut off.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mozilla-mobile/fenix/issues/8252#issuecomment-784960158, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB46VXSXUXFFFCCNZNW7AYDTATFLLANCNFSM4KR64JVQ .

smarkwell commented 3 years ago

I documented steps to reproduce on a Pixel 4a here, https://github.com/mozilla-mobile/fenix/issues/17767

freebrowser1 commented 3 years ago

You can customize the css per site using ublock origin, which can be enabled in Firefox Fenix.

https://www.reddit.com/r/firefox/comments/8w9nqm/ublock_can_style_css_you_dont_need_a_separate/

welcometochristown commented 3 years ago

To follow on from @freebrowser1 this is the filter you can add to uBlock Origin to temporarily fix the issue with youtube:

m.youtube.com##.html5-video-player.ytp-large-width-mode video.video-stream.html5-main-video:style(height: 100vh !important)

I am using a OnePlus 8 (screen size 6.55 inches, ratio 20:9), but thanks to @theSdev's comment below this should now work for all devices.

Instructions: 1). Open uBlock Origin addon inside firefox (via addons), and navigate to the dashboard (little cogs icon). 2). Select 'My filters' from the tabs on the top. 3). Copy the filter above exactly on a new line and click the tick button to save. 4). Reload your youtube video.

theSdev commented 3 years ago

The uBlock snippet works like a charm, thank y'all.

I replaced 378px with 100vh, which means the available viewport height and should work universally. So to repeat with this modification:

m.youtube.com##.html5-video-player.ytp-large-width-mode video.video-stream.html5-main-video:style(height: 100vh !important)

welcometochristown commented 3 years ago

Nice one @theSdev, good call with the 100vh.

I'll update my post with that too just incase someone copies it instead of yours :)

jxu commented 3 years ago

works for me after I tap the screen once in fullscreen mode

On Fri, Feb 26, 2021, 12:57 AM welcometochristown notifications@github.com wrote:

Nice one @theSdev https://github.com/theSdev, good call with the 100vh.

I'll update my post with that too just incase someone copies it instead of yours :)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mozilla-mobile/fenix/issues/8252#issuecomment-786431184, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB46VXV36I57DBSZ3ETOHWDTA4Z6NANCNFSM4KR64JVQ .

theSdev commented 3 years ago

And this one should work for captions:

m.youtube.com##.html5-video-player.ytp-large-width-mode .captions-text:style(position: fixed; bottom: 1em)

I'm clearly too excited about this workaround 😁

escape0707 commented 3 years ago

Thanks guys! You guys are amazing!

I personally tweaked the above workarounds into this, and think it's sweet. I'm a CSS noob so please correct me if I'm doing anything in the wrong way.

The below filter could possibly compensate YouTube side video zooming problem and the subtitle misplaced problem at once, with subtitles scrolling remained, for now.

m.youtube.com##.ytp-fullscreen .html5-main-video:style(height: 100vh !important;)
m.youtube.com##.ytp-fullscreen .caption-window:style(position: fixed !important;)
ghajini commented 3 years ago

This is not just YouTube, vimeo issue, This covers all other video playing sites See

https://github.com/webcompat/web-bugs/issues/67897 https://github.com/webcompat/web-bugs/issues/67201

emilio commented 3 years ago

Well, I think we should split this issue a bit, as those are clearly different bugs, and those need more investigation...

I looked at webcompat/web-bugs#67897, and that is because we're using a desktop viewport, and the fullscreen code seems confused. I guess that could explain a bunch of the other "long tail" of issues, actually. But it's definitely different from the youtube / vimeo issues. That at least seems like a Firefox issue... I'll try to figure out a reduced test-case for that.

emilio commented 3 years ago

So yeah for that website we're using a viewport of 980x480 (the desktop viewport), and that when fullscreen is obviously not going to work. The site could fix that by adding something like <meta name=viewport content="width=device-width"> but in this case Firefox should do the right thing automatically.

I wrote a reduced test-case and bug report here https://bugzilla.mozilla.org/show_bug.cgi?id=1696717. I believe that may be the cause of most of the other issues, at least for non-mobile-friendly pages. The links in webcompat/web-bugs#67201 are dead, but from the screenshots it's probably the same issue. I'll try to get that bug prioritized or get to it myself sometime soon.

Thanks for raising that @ghajini, much appreciated.

ghajini commented 3 years ago

Here is link https://www58.zippyshare.com/v/eRMO1pAn/file.html for the report https://github.com/webcompat/web-bugs/issues/67201

emilio commented 3 years ago

That gives a 403 error for me.

ghajini commented 3 years ago

That works on my end or try one of the links below ,they all work for me @emilio

expand to see links ``` https://www27.zippyshare.com/v/rYWtimII/file.html https://www9.zippyshare.com/v/tUOO0EG3/file.html https://www32.zippyshare.com/v/ne1tv5ko/file.html https://www1.zippyshare.com/v/F41RvxqG/file.html https://www92.zippyshare.com/v/xHvZWBMc/file.html ```
jxu commented 3 years ago

you can upload directly to github issue tracker. no need for random file sharing sites.

ghajini commented 3 years ago

https://github.com/webcompat/web-bugs/issues/67788

Similar bug ie video is overzoomed

Pinging you @karlcow @hiikezoe so that you communicate well with @emilio

As there are more than 1 bugzilla reports opened for same bug

Also this happen with site eg https://speedostream.com/embed-f8kj1y6wwciw.html

emilio commented 3 years ago

Yeah, so that one should be a dupe of bug 1696717. So to recap, there are at least three bugs involved:

So if everything goes well and the patch is correct it should hopefully land soonish and then Nightly should have a bunch of these fixed. I'll nag YouTube / Vimeo because I haven't heard back from them.

emilio commented 3 years ago

I filed https://github.com/whatwg/fullscreen/issues/191 on getting the "viewport meta etc is ignored if fullscreen" specified.

emilio commented 3 years ago

I manually confirmed that my patch fixed both webcompat/web-bugs#67897 and webcompat/web-bugs#67788 (as expected)

emilio commented 3 years ago

Latest android nightly should have the fix I wrote, so a bunch of the pages here should be fixed. Vimeo have acknowledged the bug, and YouTube folks haven't replied yet... :(

ghajini commented 3 years ago

At this website

https://aparat.cam/embed-x4v7e0z734dj.html

Both chrome/Firefox android overzoom the video in PORTRAIT mode (Try at 16 sec of video from start, subtitles /headings are cut )

Is it site issue? But one should be able to see the video efficiently

emilio commented 3 years ago

Yes, the site is using object-fit: contain; but also transform: matrix(1.15, 0, 0, 1.15, 0, 0);, so they're scaling the video up in a way that it overflows the viewport if your screen is too wide. Removing that transform fixes it. As to why they're doing that, I haven't dug, but yeah it seems to be a site issue.

freebrowser1 commented 3 years ago

Tested on Fenix Beta 87.0 using 'desktop' user agent.

Indeed the subtitle falls halfway under the bottom (using Landscape mode).

escape0707 commented 3 years ago

@freebrowser1 You mean youtube? Have you tried this? https://github.com/mozilla-mobile/fenix/issues/8252#issuecomment-787394530

RByers commented 3 years ago

Sorry for the trouble, UA sniffing sucks! I was able to reproduce this with the first video I tried on my Pixel 5. I have filed a Google internal bug on YouTube for this, b/182994254, and asked the YouTube team to follow up here if possible. I hope it helps!

emilio commented 3 years ago

Thanks @RByers!

pa3m3r commented 3 years ago

Found a fix on Galaxy S21: Get the YouTube High Definition add-on Go to the settings and choose Expand for Video Size For it to work the phone has to be on landscape mode when you fullscreen the youtube video

AlexEastwood commented 3 years ago

Fix for Moto g9 Plus: Go to Settings > Display > Advanced > Full Screen > Turn on Firefox

github-monkey commented 3 years ago

I'm experiencing a possibly related issue: the YouTube touch zones for the timeline and controls don't line up in full screen landscape in Firefox for Android on tablet. This does not happen on the phone.

When tapping the playback controls below the timeline, 9 times out of 10 I hit the timeline instead. When I try to tap Play/Pause the video instead skips to wherever I hit the timeline somewhere near the beginning. The same for Fullscreen, but I wind up with the video skipping to somewhere near the end.

I don't have uBlock but AdGuard AdBlocker, which doesn't accept the custom filter as provided earlier in this thread.

I don't have fat fingers or motor control issues. This phenomenon does not occur in fullscreen portrait, even though the controls are just as tightly situated along the bottom edge of the screen. In portrait mode the touch zones line up perfectly. This is not a general display issue or a matter of calibration. Touch tracks well in paint applications.