Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Width and height safeguards hide audio controls + video #94

Open
JayPanoz opened this issue Sep 7, 2020 · 3 comments
Open

Width and height safeguards hide audio controls + video #94

JayPanoz opened this issue Sep 7, 2020 · 3 comments
Assignees
Labels
Bug Indicates the issue is a bug Proposed solution Issues or feature requests with a solution in reach

Comments

@JayPanoz
Copy link
Collaborator

JayPanoz commented Sep 7, 2020

Submitting a bug report that was discovered by @aferditamuriqi

Short description of the issue/suggestion:

There is a rendering issue with audio and video because of width: auto and height: auto in this declaration (should also impact the vertical writing version of the module).

img, svg, audio, video {
/* We probably don’t need to use modern box-sizing as auto behaves like it */
box-sizing: var(--RS__boxSizingMedia);
width: auto;
height: auto;
/* Some files don’t have max-width */
max-width: var(--RS__maxMediaWidth);
/* We’re setting a max-height, especially for covers */
max-height: var(--RS__maxMediaHeight) !important;
/* Object-fit allows us to keep the correct aspect-ratio */
object-fit: contain;
/* For page-break, we must use those 3
We can’t use a variable there, webkit seems to no support them for those properties */
-webkit-column-break-inside: avoid;
page-break-inside: avoid;
break-inside: avoid;
}

For instance, in this screenshot, there should be audio controls:

screen_shot_2020-09-07_at_10 42 55_am

And when you disable width and height in dev tools, they are displayed correctly:

screen_shot_2020-09-07_at_10 42 49_am

Given these can be useful for other media e.g. img, audio + video should be moved to their own specific declaration. If we want to safeguard the sizing and fall back to the browser’s default, it seems we can use the revert value:

screen_shot_2020-09-07_at_10 58 36_am

This should be fixed quickly as it’s a high priority bug.

@JayPanoz JayPanoz added the Bug Indicates the issue is a bug label Sep 7, 2020
@JayPanoz JayPanoz self-assigned this Sep 7, 2020
@danielweck
Copy link
Member

Duplicate issue moved: #100

Problematic CSS:

width: auto;
height: auto;

I have an EPUB constructed from a set of webpages, notably this one (scroll down to see the audio elements):

http://diagramcenter.org/diagram-reports/diagram-report-2019/sonification.html

As tested in Thorium, ReadiumCSS applies auto to the width and height properties, resulting in a 0px CSS box (i.e. invisible audio element despite the controls attribute).

I fixed this problem by adding the following to the author stylesheet:

audio {
width: 100%;
height: 2em;
}

I would have preferred to use inherit or initial in order to avoid "hard coding" property values (especially the em value) ... but this didn't work in the Web Inspector (Chromium).

Any idea of why auto breaks things?

@danielweck
Copy link
Member

Update: in Thorium we now inject the following CSS <style> element immediately after ReadiumCSS "before" external <link> stylesheet (which is first in the HTML head), therefore before any authored styles:

audio[controls] {
width: revert;
height: revert;
}

@JayPanoz
Copy link
Collaborator Author

Proposal: add Daniel’s snippet to the safeguards module.

@JayPanoz JayPanoz added the Proposed solution Issues or feature requests with a solution in reach label Apr 22, 2024
JayPanoz added a commit that referenced this issue May 22, 2024
Audio now on its own so that value auto for width or height doesn’t break controls. Resolves #94
@JayPanoz JayPanoz mentioned this issue May 23, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Indicates the issue is a bug Proposed solution Issues or feature requests with a solution in reach
Projects
None yet
Development

No branches or pull requests

2 participants