Fullscreen video on desktop Youtube, southpark cc not fullscreen, controls broken
Categories
(Core :: Layout, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr68 | --- | unaffected |
firefox-esr78 | --- | unaffected |
firefox79 | --- | unaffected |
firefox80 | --- | wontfix |
firefox81 | --- | fixed |
firefox82 | --- | verified |
People
(Reporter: liuche, Assigned: botond)
References
(Regression)
Details
(Keywords: regression, webcompat:needs-diagnosis, Whiteboard: [geckoview])
Attachments
(2 files)
(deleted),
image/jpeg
|
Details | |
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
|
Details |
Filed from Github issues: https://github.com/mozilla-mobile/fenix/issues/13773 and https://github.com/mozilla-mobile/fenix/issues/13790
Sounds like regression on 80, and doesn't show up on 79 from reports
STR #1 southpark
Steps to repro:
Go to https://southpark.cc.com/full-episodes
Play video in full screen.
Expected:
Video should be play in full screen immersive mode hiding android navigation bar.
Actual behavior
In landscape Full screen mode there's a white bar in bottom and side of video and video is smaller than fullscreen. Video isn't in immersive landscape mode.
The south park cc site video seek bar, subtitle button, Full screen switch button are not responding.
STR #2 Youtube desktop
When you open Youtube in desktop browser view and switch to landscape mode, it offsets the video to the top right corner and white in the remaining spaces. Controls like seek, full-screen, etc. trigger in the normal locations as if there's no offset even of the controls show offset as well. Noticed this on both firefox beta and nightly (android).
Repro-ing on the current Nightly 8/17 w/ GV
I can repro this in desktop using responsive design mode. It think the culprit may be:
<meta name="viewport" content="width=640, user-scalable=no" id="viewport">
It seems like this is probably web compat or layout.
Reporter | ||
Comment 2•4 years ago
|
||
The user does say that Fenix 79 doesn't have this problem, which points to it being a regression rather than a webcompat issue though.
Comment 3•4 years ago
|
||
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #1)
I can repro this in desktop using responsive design mode. It think the culprit may be:
<meta name="viewport" content="width=640, user-scalable=no" id="viewport">
It seems like this is probably web compat or layout.
I wouldn't expect meta viewport to affect fullscreen video, but Karl knows more about viewport than I do. Karl, ever seen anything like this?
Comment 4•4 years ago
|
||
[EdgePlayer] ERROR [177] Error in "FULL_SCREEN_CHANGE" callback. FULL_SCREEN_CHANGE@https://southpark.cc.com/sitewide/js/app/main-packaged.js:15130:38
trigger/<@https://player.mtvnservices.com/edge/player/3.0.1/edgeplayer.min.js:1:555720
trigger@https://player.mtvnservices.com/edge/player/3.0.1/edgeplayer.min.js:1:555694
onFullscreenChange@https://player.mtvnservices.com/edge/player/3.0.1/edgeplayer.min.js:1:378779
E/</n.proxy@https://player.mtvnservices.com/edge/player/3.0.1/edgeplayer.min.js:1:37334
nrWrapper@https://southpark.cc.com/full-episodes/s23e10-christmas-snow:12:25380
TypeError: can't access property "style", document.querySelector(...) is null
FULL_SCREEN_CHANGE https://southpark.cc.com/sitewide/js/app/main-packaged.js:15130
trigger https://player.mtvnservices.com/edge/player/3.0.1/edgeplayer.min.js:1
trigger https://player.mtvnservices.com/edge/player/3.0.1/edgeplayer.min.js:1
onFullscreenChange https://player.mtvnservices.com/edge/player/3.0.1/edgeplayer.min.js:1
proxy https://player.mtvnservices.com/edge/player/3.0.1/edgeplayer.min.js:1
nrWrapper https://southpark.cc.com/full-episodes/s23e10-christmas-snow:12
Object { callback: FULL_SCREEN_CHANGE(event)
, context: undefined, ctx: {…} }
edgeplayer.min.js:1:17751
Uncaught TypeError: can't access property "style", document.querySelector(...) is null
FULL_SCREEN_CHANGE https://southpark.cc.com/sitewide/js/app/main-packaged.js:15130
trigger https://player.mtvnservices.com/edge/player/3.0.1/edgeplayer.min.js:1
trigger https://player.mtvnservices.com/edge/player/3.0.1/edgeplayer.min.js:1
onFullscreenChange https://player.mtvnservices.com/edge/player/3.0.1/edgeplayer.min.js:1
proxy https://player.mtvnservices.com/edge/player/3.0.1/edgeplayer.min.js:1
nrWrapper https://southpark.cc.com/full-episodes/s23e10-christmas-snow:12
main-packaged.js:15130:38
The player init is
var playerInit = function () {
VideoPlayer.init(
$.extend(
{
id: self.mgid,
size: {
width: parseInt($videoContainer.width(), 10),
height: parseInt($videoContainer.width() * (9 / 16), 10),
},
events: {
PLAYLIST_END: playlistCompleteCallback,
FULL_SCREEN_CHANGE: function (event) {
document.querySelector(".evidon-banner").style.display = event.data
? "none"
: "";
},
},
},
window.playerConfig.playerVars
),
window.playerConfig.containerID
);
var resizePlayerOnViewportSizeChange = function () {
var playerId = "#player_page_player";
var $videoPlayer = self.ui.$video.find(playerId);
var $videoPlayerIframe = $videoPlayer.children("iframe");
// No Videos Exist
if ($videoPlayer.length == 0) {
return;
}
// Resize Video
$videoPlayer.width($videoContainer.width());
$videoPlayer.height($videoContainer.width() * (9 / 16));
// Hulu
if (typeof NewSite != "undefined")
NewSite.videoPlayerComponent.setSize(
$videoContainer.width(),
$videoContainer.width() * (9 / 16)
);
};
if (mobileDetect.isIpad() || mobileDetect.isAndroidTablet()) {
$window.on("HEADER::orientationChange", function () {
resizePlayerOnViewportSizeChange();
});
} else {
// Window Resize
$window.on("WINDOW::resize", resizePlayerOnViewportSizeChange);
}
$(document).trigger("SP::PlayerInit");
};
Hiroyuki, does the viewport size change when switching to full screen on fenix?
Comment 5•4 years ago
|
||
The viewport size could be changed because we don't allow minimum-scale size on fullscreen state, I tried to drop the restriction in bug 1650686, but it's not yet happened. Anyways, the restriction is only affecting where the minimum-scale size is applied to the document, is the document having an element bigger than the initial containing block size?
That said, even if the viewport size is changed, I don't know how it's affecting the video size at the first glance of the code in comment 4.
Probably the best way to move this issue forward to find out the regression range.
Comment 6•4 years ago
|
||
On the RDM, I saw the video is rendered at the bottom on full screen state, so maybe it's not the same issue, but I narrowed it down but mozregression failed with 'taskId' error, probably it's due to bug 1655682.
Anyways the range I got is;
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=dd1766e040a2db3cf1497ffe4b221e5082a831ee&tochange=2aa3b889d60386ac20e2a7ab7f315a742a2eea87
The most suspicious one is bug 1650488. That said, the change itself looks pretty reasonable to me. So there must be something badly relying on the async zoom container in full screen stuff.
CCing kats.
Comment 7•4 years ago
|
||
I could narrow down the range further; https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=3f4573c955ea4e1f9b7ee38e0254ca3d13d095b5&tochange=7e46319259d9d69f1dcb5dbdbca0677f385ab8c6
Bug 1650488 is still there. So I am ~100% sure it's the regressioner of the RDM issue, but I still believe it's just a trigger of unwallpapering a pre-existing issue.
Another interesting thing is I can't reproduce the original issue on a local built Fenix based on m-c: 069bb8bd2356 and based on fenix: 751efb7bfbc2930fcbe5cd4a1b90f9ffca8c701e.
Updated•4 years ago
|
Comment 8•4 years ago
|
||
I could mange to reproduce the original issue on a local built Fenix on Android emulator. To reproduce the issue, I had to change the device resolution by using "adb shell wm size 1080x1920".
And dropping !document->Fullscreen()
check introduced in bug 1650488 fixes the original issue actually. Nothing more I've done so far.
Updated•4 years ago
|
Comment 9•4 years ago
|
||
Set release status flags based on info from the regressing bug 1650488
Comment 10•4 years ago
|
||
Kats, given the regressing change Hiro identified here, can you help at all?
Updated•4 years ago
|
Comment 11•4 years ago
|
||
I'll try to get to this next week if nobody else gets to it before then. I can't repro using the southpark STR because playing videos there is blocked in Canada. I see wonky behaviour on youtube but I'm not sure if it's the same as what is described in this bug. So it might be hard for me to track this down, but I'll see what I can do.
I can repro with https://www.w3.org/2010/05/video/mediaevents.html
Comment 14•4 years ago
|
||
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #12)
I can repro with https://www.w3.org/2010/05/video/mediaevents.html
What I see on this is similar to what I was seeing on youtube - when I enter fullscreen, the video is zoomed in rather than zoomed to fit. Which is clearly a bug, but sounds different from what the "actual results" in comment 0 is describing.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #14)
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #12)
I can repro with https://www.w3.org/2010/05/video/mediaevents.html
What I see on this is similar to what I was seeing on youtube - when I enter fullscreen, the video is zoomed in rather than zoomed to fit. Which is clearly a bug, but sounds different from what the "actual results" in comment 0 is describing.
Yeah, I guess I sorta think they're the same bug?
Assignee | ||
Comment 16•4 years ago
|
||
The change made in bug 1650488 is not to build an async zoom container if we're in fullscreen mode. This has the effect of not applying any APZ resolution to the page contents.
On desktop, this is fine, since you can't zoom while in fullscreen mode. However, on Android, allowing the user to zoom is not the only purpose of the APZ resolution -- it's also used to respect the initial scale computed for mobile viewport sizing purposes (commonly, on desktop sites, to zoom out such that the ICB size (which for desktop sites is usually wider than the screen) fits visually into the screen size). It appears that this remains important in fullscreen mode, too, i.e. the fullscreen page contents are still sized to the ICB size (and with a fixed width in the meta viewport tag, the ICB size does not change based on being on fullscreen mode).
So, I don't think the fix in bug 1650488 is appropriate for mobile. We may just want to restrict it to desktop only.
Assignee | ||
Comment 17•4 years ago
|
||
Comment 18•4 years ago
|
||
Thanks! Your reasoning makes sense. I'm assigning the bug to you since you already have a patch up :)
Comment 19•4 years ago
|
||
Comment 20•4 years ago
|
||
bugherder |
Reporter | ||
Comment 21•4 years ago
|
||
Since Fenix is doing a big release with 81, could we get an uplift to beta as well?
Assignee | ||
Comment 22•4 years ago
|
||
Comment on attachment 9172292 [details]
Bug 1659761 - Restrict the fix of bug 1650488 to desktop only. r=kats
Beta/Release Uplift Approval Request
- User impact if declined: On some pages on android (particularly desktop-mode sites), fullscreening a video can result in incorrect rendering (e.g. the video being sized larger than the screen such that only part of the video is visible).
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): We are just reverting a change (bug 1650488) that was motivated by desktop zooming, for android. After this change, the android behaviour will return to known-good state prior to bug 1650488.
In terms of test coverage, we have coverage for this patch not regressing bug 1650488 on desktop, but not for the issue being fixed on android.
- String changes made/needed: none
Comment 23•4 years ago
|
||
Comment on attachment 9172292 [details]
Bug 1659761 - Restrict the fix of bug 1650488 to desktop only. r=kats
Approved for 81.0b4.
Comment 24•4 years ago
|
||
bugherder uplift |
Comment 25•4 years ago
|
||
Hi, verified as fixed on the latest Nightly Build 200831 using the following devices:
• Google Pixel 3a (Android 10)
• Huawei Mate 20 Lite (Android 10)
• OnePlus A3 (Android 6.0.1)
Updated•4 years ago
|
Description
•