third_party/WebKit/LayoutTests/media/controls/buttons-after-reset.html - Issue 2704013002: Media Controls: cache the minimum width in order to reduce incorrect width usage.

Side by Side Diff

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Side by Side Diff: third_party/WebKit/LayoutTests/media/controls/buttons-after-reset.html

Issue 2704013002: Media Controls: cache the minimum width in order to reduce incorrect width usage. (Closed)

Patch Set: Created 3 years, 10 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

	2 <html>

	3 <title>Test that resetting the controls after load is a no-op.</title>

	4 <script src="../media-file.js"></script>

	5 <video controls width=400>

	6 <track kind=subtitles src=fake-en-sub.vtt srclang=en label=English>

	7 <track kind=subtitles src=fake-fr-sub.vtt srclang=fr label=French>

	8 </video>

	9 <script>

	10 if (window.testRunner)

	11 testRunner.waitUntilDone();

	12

	13 var video = document.querySelector('video');

	14 video.src = findMediaFile('video', '../content/test');

	15 video.addEventListener('loadedmetadata', () => {
	Zhiqiang Zhang (Slow) 2017/02/18 15:04:37 Can you explain why hide and show the controls and Can you explain why hide and show the controls and then check if it matches the expectation? mlamouri (slow - plz ping) 2017/02/19 10:33:57 Without the fix, when we do hide/show, we end up w Show quoted text On 2017/02/18 at 15:04:37, Zhiqiang Zhang wrote: > Can you explain why hide and show the controls and then check if it matches the expectation? Without the fix, when we do hide/show, we end up without a computed size so 48 will end up being used. With the fix, the 32 value that was computed earlier will be kept. My guess with regards to why this is happening is that when we hide the controls, obviously the style is lost. Then, we show them back but the `computeWhichControlsFit()` is called before the controls are drawn (which makes sense). Thus, they have no style and 48px will be used as the default value. Because we don't do useless layouts on purpose, we wouldn't call `computeWhichControlsFit()` again for a while. The real fix would be much complex unless we hard code the values: the issue here is that `computeWhichControlsFit()` requires a style to be available but this is called as part of the layout, creating a weird cyclical dependency. Another dirty work around would be to, on purpose, have a second layout in some situations so `computeWhichControlsFit()` would be called in a proper state the second time. Though, it might create flickering.
	16 video.controls = false;

	17 testRunner.layoutAndPaintAsyncThen(() => {

	18 video.controls = true;

	19

	20 testRunner.layoutAndPaintAsyncThen(() => {

	21 testRunner.notifyDone();

	22 });

	23 });

	24 });

	25 </script>

	26 </html>

OLD	NEW

« no previous file with comments | « no previous file | third_party/WebKit/LayoutTests/media/controls/buttons-after-reset-expected.html » ('j') | no next file with comments »