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

Implement ::backdrop pseudo-element parsing #103

Merged
merged 1 commit into from
Jan 7, 2025

Conversation

nicoburns
Copy link
Collaborator

Context

Notes

Github.com sets it's root-level CSS variables using a selector that includes a ::backdrop pseudo selector. It's not quite this, but it's morally equivalent to html, html ::backdrop. And the presence of ::backdrop causes the entire selector to fail to parse meaning that the variables never apply and styling is way off.

Changing the selector to html, html ::non-existent-selector causes the styles to fail to apply in Chrome too. So I think the strict parsing is correct per the spec. So I believe the only way to make this work is to implement parsing for ::backdrop. I don't think we support creating a backdrop in a current Servo, but if we did support then we probably would support matching styles against it, so enabling the selector seems reasonable to me.

Feedback wanted

I wasn't sure if it should be a Lazy pseudo-element. But Lazy is documented as being for regular but rarely used pseudo-elements which seemed to fit, whereas PreComputed is documented as being only for internal pseudo-elements which didn't seem to fit.

@nicoburns nicoburns added the enhancement New feature or request label Jan 6, 2025
Copy link
Contributor

@Loirooriol Loirooriol left a comment

Choose a reason for hiding this comment

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

I think we don't support modal dialogs nor fullscreen, so it shouldn't be observable whether we support ::backdrop or not, thus claiming we do isn't wrong :P

Not much sure about the pseudo cascade type either, but lazy sounds reasonable.

@jdm
Copy link
Member

jdm commented Jan 6, 2025

I think we don't support modal dialogs nor fullscreen, so it shouldn't be observable whether we support ::backdrop or not, thus claiming we do isn't wrong :P

Not much sure about the pseudo cascade type either, but lazy sounds reasonable.

We do support the DOM full screen API, unless you're referring to something else.

@nicoburns
Copy link
Collaborator Author

nicoburns commented Jan 6, 2025

Does the fullscreen API use ::backdrop? I thought it was just "top layer" that used backdrops. I wouldn't expect fullscreen to have a backdrop because, well, it takes up the whole screen so you won't be be able to see anything behind.

@nicoburns nicoburns added this pull request to the merge queue Jan 7, 2025
Merged via the queue into servo:main with commit aab832f Jan 7, 2025
3 checks passed
@nicoburns nicoburns deleted the backdrop-psuedo branch January 7, 2025 01:36
@Loirooriol
Copy link
Contributor

Yes, the fullscreen API places elements in the top layer, in fact ::backdrop was initially defined in the fullscreen spec: https://fullscreen.spec.whatwg.org/#css-pe-backdrop

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

Successfully merging this pull request may close these issues.

Style resolution issues on Github.com
3 participants