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

The "acid" test is not specific enough (not "unit" testing) + fails with typical stylesheet overrides from reading system "user agent" and/or user formatting preferences #164

Closed
danielweck opened this issue Jun 15, 2022 · 6 comments · Fixed by #167

Comments

@danielweck
Copy link
Member

Screenshot from Apple iBooks / MacOS Books.app (same broken rendition with Thorium):

Screenshot 2022-06-15 at 14 34 52

The rationale for including the Acid test in the EPUB test suite was mentioned in this PR #7

I also made a version of the Acid2 test, as that seemed one way to test the requirement about reading systems with a viewport: "If it has a Viewport, it MUST support visual rendering of XHTML Content Documents as defined in § 4.3.1 CSS Conformance."

This Acid test was indeed a useful exercise that offered a vivid visual demonstration of the shortcomings of some competing CSS implementations in web browsers and other rendering engines. However I think the value of Acid test in the context of the EPUB test suite is arguably debatable because it is not clear what feature is tested exactly. Furthermore, reading systems typically apply a default "user agent" stylesheet (+ additional overrides based on user preferences) that cause different rendering than vanilla web browsers. For example, pagination using CSS columns usually involves overriding margins, etc.

My recommendation would be to eliminate the Acid test from this test suite.

@danielweck
Copy link
Member Author

CC @llemeurfr @gautierchomel

@iherman
Copy link
Member

iherman commented Jun 16, 2022

cc @dauwhe @dlazin

@dauwhe
Copy link
Contributor

dauwhe commented Jun 16, 2022

My recommendation would be to eliminate the Acid test from this test suite.

I'm fine with that. As I mentioned, it was one way of showing that an RS supports "a lot" of CSS, but it is a strange test (and had to be edited to be XHTML, etc.)

@iherman
Copy link
Member

iherman commented Jun 17, 2022

I have created a PR to do so (#167).

However, we have now a MUST statement on being conform to CSS that has now corresponding test (actually, we have the statement twice: one in the introductory statement in the paragraph and then one in the core text of §6.3. This contradicts to the requirement for having a test for each MUST statement. I am not sure whether this is a problem and what we should put there instead.

(Almost all our tests use CSS in some way. One could argue that CSS support is amply shown through the test suite... but maybe removing the first bullet item, which is a repetition of what is said in the yellow banner, makes things a big cleaner. This is reminiscent on the discussion in w3c/epub-specs#2330)

cc @dlazin @mattgarrish

@dlazin
Copy link
Collaborator

dlazin commented Jun 17, 2022

I agree with removing this; I have similar thoughts, but haven't found time to create a replacement test. Note that I think it's fine to use a basic test for two different normative statements, if it reasonably tests the same things; not as good as having unique tests, but IMO fine. I don't think there's a meaningful difference between "you must support rendering CSS" and "you must support CSS as defined in the CSS spec"; one basic test seems sufficient to me.

@iherman
Copy link
Member

iherman commented Jun 17, 2022

one basic test seems sufficient to me.

me too. But, with the removal of this test, there is no test for the statements in §6.3. Even if we decide to remove the bullet item:

MUST support the official definition of CSS as described in the [csssnapshot].

(which I would agree with), we may still to either have, or assign another test, for the statement in the intro part of the section, i.e.,

If a reading system has a viewport, it MUST support the visual rendering of XHTML content documents via CSS [epub-33].

So far, the acid test was used for both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants