Skip to content

Commit b0f444c

Browse files
Ahmad-S792Ahmad Saleem
Ahmad-S792
authored and
Ahmad Saleem
committed
Fix double-zoom issue with video poster images
https://bugs.webkit.org/show_bug.cgi?id=292415 rdar://150976146 Reviewed by Brent Fulgham. Inspired by: https://chromium.googlesource.com/chromium/src/+/75dc85b2a87137b90854ff8b3a16ed6da5081ac8 m_cachedImageSize already contains zoom-scaled dimensions when cached in imageChanged(). Applying zoom again in calculateIntrinsicSize() caused poster images to be incorrectly double-scaled. Move poster size handling to calculateIntrinsicSize() to return the cached size directly without additional scaling. Only apply zoom to unscaled sizes from calculateIntrinsicSizeInternal(). * Source/WebCore/rendering/RenderVideo.cpp: (WebCore::RenderVideo::calculateIntrinsicSizeInternal): (WebCore::RenderVideo::calculateIntrinsicSize): * LayoutTests/fast/css/video-poster-zoom-expected.txt: Added. * LayoutTests/fast/css/video-poster-zoom.html: Added. Canonical link: https://commits.webkit.org/305347@main
1 parent 4863df9 commit b0f444c

File tree

3 files changed

+126
-12
lines changed

3 files changed

+126
-12
lines changed

‎LayoutTests/fast/css/video-poster-zoom-expected.txt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
2+
PASS Video poster size with zoom:1
3+
PASS Video poster size with zoom:1.5
4+
PASS Video poster size with zoom:2 and non-square dimensions
5+

‎LayoutTests/fast/css/video-poster-zoom.html‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8">
5+
<title>Video poster image should respect CSS zoom</title>
6+
<script src="../../resources/testharness.js"></script>
7+
<script src="../../resources/testharnessreport.js"></script>
8+
<style>
9+
video {
10+
/* Ensure no default styling interferes. */
11+
border: none;
12+
padding: 0;
13+
margin: 0;
14+
}
15+
</style>
16+
</head>
17+
<body>
18+
<script>
19+
// Helper to create a data URL image of specific size.
20+
function createImageDataURL(width, height) {
21+
const canvas = document.createElement('canvas');
22+
canvas.width = width;
23+
canvas.height = height;
24+
const ctx = canvas.getContext('2d');
25+
// Fill with a color so it's visible.
26+
ctx.fillStyle = 'blue';
27+
ctx.fillRect(0, 0, width, height);
28+
return canvas.toDataURL();
29+
}
30+
31+
promise_test(async t => {
32+
const video = document.createElement('video');
33+
video.poster = createImageDataURL(10, 10);
34+
video.style.zoom = '1';
35+
document.body.appendChild(video);
36+
37+
// Wait for poster to load.
38+
await new Promise(resolve => {
39+
const img = new Image();
40+
img.onload = resolve;
41+
img.src = video.poster;
42+
});
43+
44+
// Force layout.
45+
video.getBoundingClientRect();
46+
47+
const rect = video.getBoundingClientRect();
48+
assert_equals(rect.width, 10, 'Video width should match poster width with zoom:1');
49+
assert_equals(rect.height, 10, 'Video height should match poster height with zoom:1');
50+
51+
video.remove();
52+
}, 'Video poster size with zoom:1');
53+
54+
promise_test(async t => {
55+
const video = document.createElement('video');
56+
video.poster = createImageDataURL(10, 10);
57+
video.style.zoom = '1.5';
58+
document.body.appendChild(video);
59+
60+
// Wait for poster to load.
61+
await new Promise(resolve => {
62+
const img = new Image();
63+
img.onload = resolve;
64+
img.src = video.poster;
65+
});
66+
67+
// Force layout.
68+
video.getBoundingClientRect();
69+
70+
const rect = video.getBoundingClientRect();
71+
assert_equals(rect.width, 15, 'Video width should be scaled by zoom:1.5');
72+
assert_equals(rect.height, 15, 'Video height should be scaled by zoom:1.5');
73+
74+
video.remove();
75+
}, 'Video poster size with zoom:1.5');
76+
77+
promise_test(async t => {
78+
const video = document.createElement('video');
79+
video.poster = createImageDataURL(20, 10);
80+
video.style.zoom = '2';
81+
document.body.appendChild(video);
82+
83+
// Wait for poster to load.
84+
await new Promise(resolve => {
85+
const img = new Image();
86+
img.onload = resolve;
87+
img.src = video.poster;
88+
});
89+
90+
// Force layout.
91+
video.getBoundingClientRect();
92+
93+
const rect = video.getBoundingClientRect();
94+
assert_equals(rect.width, 40, 'Video width should be scaled by zoom:2');
95+
assert_equals(rect.height, 20, 'Video height should be scaled by zoom:2');
96+
97+
video.remove();
98+
}, 'Video poster size with zoom:2 and non-square dimensions');
99+
</script>
100+
</body>
101+
</html>

‎Source/WebCore/rendering/RenderVideo.cpp‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -127,24 +127,13 @@ LayoutSize RenderVideo::calculateIntrinsicSizeInternal()
127127
Ref videoElement = this->videoElement();
128128
RefPtr player = videoElement->player();
129129

130-
// Determine what we should display: poster or video
131-
// If the show-poster-flag is set (or there is no video frame to display) AND
132-
// there is a poster image, display the poster.
133-
bool shouldUsePoster = (videoElement->shouldDisplayPosterImage() || !player || !player->hasAvailableVideoFrame()) && hasPosterFrameSize();
134-
if (shouldUsePoster)
135-
return m_cachedImageSize;
136-
137-
// Otherwise, the intrinsic width is that of the video.
130+
// Assume the intrinsic width is that of the video.
138131
if (player && videoElement->readyState() >= HTMLVideoElement::HAVE_METADATA) {
139132
LayoutSize size(player->naturalSize());
140133
if (!size.isEmpty())
141134
return size;
142135
}
143136

144-
// Fallback to poster if we have it (no video metadata yet).
145-
if (hasPosterFrameSize())
146-
return m_cachedImageSize;
147-
148137
// <video> in standalone media documents should not use the default 300x150
149138
// size since they also have audio-only files. By setting the intrinsic
150139
// size to 300x1 the video will resize itself in these cases, and audio will
@@ -160,6 +149,25 @@ LayoutSize RenderVideo::calculateIntrinsicSize()
160149
if (shouldApplySizeContainment())
161150
return intrinsicSize();
162151

152+
// Return cached poster size directly if we're using it, since it's already scaled.
153+
// Determine what we should display: poster or video.
154+
// If the show-poster-flag is set (or there is no video frame to display) AND
155+
// there is a poster image, display the poster.
156+
Ref videoElement = this->videoElement();
157+
RefPtr player = videoElement->player();
158+
bool shouldUsePoster = (videoElement->shouldDisplayPosterImage() || !player || !player->hasAvailableVideoFrame()) && hasPosterFrameSize();
159+
160+
if (shouldUsePoster) {
161+
auto cachedSize = m_cachedImageSize;
162+
if (shouldApplyInlineSizeContainment()) {
163+
if (isHorizontalWritingMode())
164+
cachedSize.setWidth(intrinsicSize().width());
165+
else
166+
cachedSize.setHeight(intrinsicSize().height());
167+
}
168+
return cachedSize;
169+
}
170+
163171
auto calculatedIntrinsicSize = calculateIntrinsicSizeInternal();
164172
calculatedIntrinsicSize.scale(style().usedZoom());
165173

0 commit comments

Comments
 (0)