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

Added CSS 2 #222

Merged
merged 4 commits into from
Jan 15, 2022
Merged

Added CSS 2 #222

merged 4 commits into from
Jan 15, 2022

Conversation

SebastianZ
Copy link
Collaborator

This change adds everything introduced in CSS 2 Revision 1. It also moves tests of things into CSS 2 that were initially introduced there but were tested in other specs so far.

The tests include the deprecated clip property because browsers are still required to support it.

I skipped all aural properties like speak, cue, volume, etc. because they are non-normative and removed in the newest working draft of the spec.

Note that this should wait until #212 is merged, so we have a way to filter CSS 2 out. Though you can already review all the changes made. In the end I'll just add a filter that includes every spec. including CSS 2.

This closes #208.

Sebastian

@SebastianZ SebastianZ marked this pull request as draft January 10, 2022 21:33
@PolariTOON
Copy link
Collaborator

I have not reviewed all the changes yet, but two things already come to my mind:

  • Since the published version of CSS2 is still a multipage spec, somewhat massive compared to individual CSS3 modules, could you split it in several virtual subspecs, one for each section, like what i did for SVG2? This way, features would be grouped by topic.
  • As you said in the issue thread, we should likely refer to the "CSS2.2" revision for the published version for now, since the version you currently linked is not updated anymore and has this big warning message when we open it?

@SebastianZ SebastianZ force-pushed the css2 branch 3 times, most recently from 36f3b7f to 301aa53 Compare January 13, 2022 20:43
@SebastianZ
Copy link
Collaborator Author

@PolariTOON Thank you for the review!

I've now split up the specification into smaller ones covering the different parts as you suggested.
And I am also referring to CSS 2.2 in the links now.

Could you please make another review?

Sebastian

Copy link
Collaborator

@PolariTOON PolariTOON left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I spotted some nits and maybe basic selectors and media queries could be tested, but overall this looks good.

tests.js Outdated Show resolved Hide resolved
tests.js Outdated Show resolved Hide resolved
tests.js Outdated Show resolved Hide resolved
tests.js Outdated Show resolved Hide resolved
tests.js Show resolved Hide resolved
tests.js Outdated Show resolved Hide resolved
tests.js Show resolved Hide resolved
tests.js Outdated Show resolved Hide resolved
@SebastianZ SebastianZ requested a review from PolariTOON January 14, 2022 12:55
@SebastianZ
Copy link
Collaborator Author

@PolariTOON Thank you for the detailed review! I've addressed everything you've mentioned. Could you please review it again?

Sebastian

Copy link
Collaborator

@PolariTOON PolariTOON left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. It's interesting to see that some features that were introduced right back in CSS2 are still not fully supported nowadays in major browsers: orphans, widows, outline-color: invert...

@SebastianZ SebastianZ marked this pull request as ready for review January 15, 2022 18:31
@SebastianZ SebastianZ merged commit fb99787 into gh-pages Jan 15, 2022
@SebastianZ
Copy link
Collaborator Author

LGTM.

Thank you again for the reviews!

It's interesting to see that some features that were introduced right back in CSS2 are still not fully supported nowadays in major browsers: orphans, widows, outline-color: invert...

Yes, interestingly, according to MDN web docs it seems that outline-color: invert was implemented at some point though support for it got removed again from browsers.

And I just realized again why I didn't include the keyword initially. The spec. says this:

Conformant UAs may ignore the invert value on platforms that do not support color inversion of the pixels on the screen. If the UA does not support the invert value then the initial value of the outline-color property is the value of the color property, similar to the initial value of the border-top-color property.

So, as I merged the PR already, I guess I should create a new PR to remove the test for it again.

Sebastian

@SebastianZ
Copy link
Collaborator Author

And btw., the implementation of widows and orphans in Gecko is tracked in https://bugzil.la/137367, if you're interested in following it.

@LeaVerou There we have something that's unimplemented for 20 years. And I am sure they will be implemented at some point because Firefox is the only browser that doesn't support those properties and there is still author interest for them.

Sebastian

@SebastianZ
Copy link
Collaborator Author

And I just realized again why I didn't include the keyword initially. The spec. says this:

Conformant UAs may ignore the invert value on platforms that do not support color inversion of the pixels on the screen. If the UA does not support the invert value then the initial value of the outline-color property is the value of the color property, similar to the initial value of the border-top-color property.

I just removed it again in 93faed6.

Sebastian

@SebastianZ SebastianZ deleted the css2 branch January 27, 2022 21:10
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 this pull request may close these issues.

Should CSS 2 be part of this test suite?
3 participants