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

Test computed styles from user agent stylesheet? #5625

Open
domenic opened this issue Apr 20, 2017 · 8 comments
Open

Test computed styles from user agent stylesheet? #5625

domenic opened this issue Apr 20, 2017 · 8 comments

Comments

@domenic
Copy link
Member

domenic commented Apr 20, 2017

Inspired by whatwg/html#2561 (comment).

The HTML Standard contains a supposedly-normative user agent stylesheet. I've also seen non-normative appendices in various CSS specs; not sure what that's about.

In theory this should be testable by using getComputedStyle() on various elements.

In practice I know Blink's UA stylesheet doesn't match the spec super-great. I can't imagine others do either (maybe Servo?). In a large part this is because some of the stuff the spec specifies in CSS is implemented in non-CSS ways. (See one attempt to align in whatwg/html#2298; I should probably get back to that...)

Is this an area worth investing in tests for, and trying to align browsers?

@fantasai
Copy link
Contributor

There are some normative statements about default UA rules in CSS specs as well. Typically new stuff that isn't stable enough yet to ask HTML to add to its stylesheet, or stuff that goes into SVG's style sheet rather than HTML's.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed testing user agent stylesheets.

The full IRC log of that discussion <astearns> topic: testing user agent stylesheets
<astearns> https://github.com//issues/5625
<TabAtkins> GitHub topic: https://github.com//issues/5625
<surma> astearns: Should we have tests for UA stylesheets?
<surma> dbaron: It’s a good idea. But some results might be an issue against the HTML spec rather than the UA
<surma> fantasai: Maybe the UA stylesheet tests should migrate to HTML
<fantasai> s/tests/statements/
<dbaron> xidorn: test cascade level?
<dbaron> dbaron: There are a lot of known issues where what's in the UA level vs. the preshint level doesn't match the spec, since the reverse engineering didn't really consider that, I don't think.
<surma> astearns: so general agreement?
<surma> [yes]

@zcorpan
Copy link
Member

zcorpan commented Apr 21, 2017

<dbaron> dbaron: There are a lot of known issues where what's in the UA level vs. the preshint level

@dbaron HTML's rendering section differentiates UA styles and preshints. I'm not sure if they all match implementations, and it's a bit annoying to test since you need a user stylesheet to tell the difference I believe.

@domenic
Copy link
Member Author

domenic commented Jul 15, 2019

I'd like to ask the WPT/CSS community's opinion on how to write tests for this sort of thing. I have two ideas so far:

  • Reftests. E.g. take a <span> (or maybe better, a <custom-undefined-element>?) and style it according to the UA stylesheet. Then compare it to an unstyled instance of the element-under-test.

  • getComputedStyle() tests. I guess the idea here would be to do a similar setup, then do a deep comparison of the getComputedStyle() results for the manually-styled element vs. the element-under-test.

I am leaning toward the second option here.

@domenic
Copy link
Member Author

domenic commented Jul 15, 2019

Note: I realized that computedStyleMap() tests are better for this purpose than getComputedStyle() tests.

@zcorpan
Copy link
Member

zcorpan commented Aug 22, 2019

I've tried to attempt this several times in the past, but hadn't found a way that would scale usefully. But now I might be on to something. Check this out:

70b4ec4

The CSS can be taken verbatim from the spec (or imported to wpt like IDL files are).

Then the @namespace rules need to be changed so that instead of applying to the HTML namespace, they apply to a bogus namespace (urn:not-html).

The test includes the HTML elements we want to test. A script clones those elements, where the clones have the bogus namespace, and thus the CSS only apply to the clones, and any CSS property not included will have the CSS initial value.

The test could then compare all of the CSS properties that we're interested in comparing.

Problems:

  • The HTML spec uses stuff like :matches() that doesn't seem to be supported cross-browser. This could be solved by post-processing the CSS, or by changing the spec to not use stuff that isn't cross-browser, or by having browsers implement the missing stuff.
  • Some style requirements are stated in prose instead of as CSS. Such cases could be excluded from this setup, and have separate test files that do what they need to do to test the given requirement.
  • The spec doesn't always match what browsers implement. The tests would make this very clear and we can just fix the spec.

@domenic
Copy link
Member Author

domenic commented Aug 22, 2019

That's a pretty neat trick! To be clear, 70b4ec4 only tests a subset of properties, but you'd be proposing testing them all, right? Cf.

testToastElement((toast) => {
toast.open = true;
const mockToast = document.createElement('span');
mockToast.id = 'mock-toast-open';
mockToast.textContent = 'Message';
const mockStyler = document.createElement('style');
mockStyler.textContent = `
#mock-toast-open {
position: fixed;
bottom: 1em;
right: 1em;
border: solid;
padding: 1em;
background: white;
color: black;
z-index: 1;
}`;
document.querySelector('main').appendChild(mockStyler);
document.querySelector('main').appendChild(mockToast);
assertComputedStyleMapsEqual(toast, mockToast);
}, 'the computed style map of an open unstyled toast is the same as a span given toast defaults');
and
export const assertComputedStyleMapsEqual = (element1, element2) => {
assert_greater_than(element1.computedStyleMap().size, 0);
for (const [styleProperty, baseStyleValues] of element1.computedStyleMap()) {
const refStyleValues = element2.computedStyleMap().getAll(styleProperty);
assert_equals(baseStyleValues.length, refStyleValues.length, `${styleProperty} length`);
for (let i = 0; i < baseStyleValues.length; ++i) {
const baseStyleValue = baseStyleValues[i];
const refStyleValue = refStyleValues[i];
assert_equals(baseStyleValue.toString(), refStyleValue.toString(), `diff at value ${styleProperty}`);
}
}
}

The HTML spec uses stuff like :matches() that doesn't seem to be supported cross-browser. This could be solved by post-processing the CSS, or by changing the spec to not use stuff that isn't cross-browser, or by having browsers implement the missing stuff.

Post-processing seems reasonable, or just manually fixing the tests up as necessary.

@zcorpan
Copy link
Member

zcorpan commented Aug 22, 2019

Yes, the test I wrote now is a small subset, but I think the approach can scale to all or almost all of the CSS rules in the HTML Rendering section, and all HTML elements (across multiple test files, to not end up with too many tests in one file).

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

No branches or pull requests

6 participants