readium / readium-css

🌈 A set of reference stylesheets for EPUB Reading Systems, starting with Readium Mobile
https://readium.org/readium-css/
BSD 3-Clause "New" or "Revised" License
90 stars 20 forks source link

Width and height safeguards hide audio controls + video #94

Open JayPanoz opened 3 years ago

JayPanoz commented 3 years ago

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).

https://github.com/readium/readium-css/blob/583011453612e6f695056ab6c086a2c4f4cac9c0/css/src/modules/ReadiumCSS-safeguards.css#L48-L69

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.

danielweck commented 2 years ago

Duplicate issue moved: https://github.com/readium/readium-css/issues/100

Problematic CSS: https://github.com/readium/readium-css/blob/583011453612e6f695056ab6c086a2c4f4cac9c0/css/src/modules/ReadiumCSS-safeguards.css#L52-L53

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 commented 2 years ago

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 commented 2 months ago

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