Skip to content

Make ::placeholder public and add property restriction for ::placeholder and ::marker#322

Merged
Loirooriol merged 1 commit intoservo:mainfrom
stevennovaryo:placeholder-public
Mar 8, 2026
Merged

Make ::placeholder public and add property restriction for ::placeholder and ::marker#322
Loirooriol merged 1 commit intoservo:mainfrom
stevennovaryo:placeholder-public

Conversation

@stevennovaryo
Copy link
Copy Markdown
Member

@stevennovaryo stevennovaryo commented Mar 6, 2026

The ::placeholder pseudo element should be a public pseudo element. And, both ::placeholder and ::marker should have a property restriction as defined in the spec. In stylo, the property restriction has been computed in PropertyFlags.

Servo PR: servo/servo#43053

Copy link
Copy Markdown
Collaborator

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

Please open a Servo PR to run tests.

Comment thread style/servo/selector_parser.rs Outdated
Comment on lines +277 to +281
match self {
PseudoElement::Marker => Some(PropertyFlags::APPLIES_TO_MARKER),
PseudoElement::Placeholder => Some(PropertyFlags::APPLIES_TO_PLACEHOLDER),
_ => None,
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit to avoid repeating Some() as we add more of these:

Suggested change
match self {
PseudoElement::Marker => Some(PropertyFlags::APPLIES_TO_MARKER),
PseudoElement::Placeholder => Some(PropertyFlags::APPLIES_TO_PLACEHOLDER),
_ => None,
}
Some(match self {
PseudoElement::Marker => PropertyFlags::APPLIES_TO_MARKER,
PseudoElement::Placeholder => PropertyFlags::APPLIES_TO_PLACEHOLDER,
_ => return None,
})

Comment thread style/servo/selector_parser.rs Outdated
pub fn property_restriction(&self) -> Option<PropertyFlags> {
None
match self {
PseudoElement::Marker => Some(PropertyFlags::APPLIES_TO_MARKER),
Copy link
Copy Markdown
Collaborator

@Loirooriol Loirooriol Mar 6, 2026

Choose a reason for hiding this comment

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

For debugging, would be useful to add if static_prefs::pref!("layout.css.marker.restricted") like Gecko:

PseudoElement::Marker if static_prefs::pref!("layout.css.marker.restricted") => {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Looks like defined only in Gecko. How do we add the pref for Servo Stylo? It is in stylo_static_prefs\src\lib.rs?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes, just add it there with a default value of true

Signed-off-by: stevennovaryo <steven.novaryo@gmail.com>
@Loirooriol Loirooriol force-pushed the placeholder-public branch from 9ecf187 to 5bbd659 Compare March 7, 2026 13:14
@Loirooriol
Copy link
Copy Markdown
Collaborator

I have force-pushed this after syncing main from upstream.

@Loirooriol Loirooriol added this pull request to the merge queue Mar 8, 2026
Merged via the queue into servo:main with commit 07364a6 Mar 8, 2026
5 checks passed
github-merge-queue Bot pushed a commit to servo/servo that referenced this pull request Mar 8, 2026
…`::placeholder` and `::marker` (#43053)

Servo side of servo/stylo#322.

Update the WPT expectation following the changes to make `::placeholder`
public and adding property restriction to `::placeholder` and
`::marker`. Additionally ensure that `getComputedStyle` works for
`::placeholder`.

Testing: Existing WPTs
Fixes: #43034 
Fixes: #43035 
Fixes: #19808

Signed-off-by: Jo Steven Novaryo <steven.novaryo@gmail.com>
lando-worker Bot pushed a commit to mozilla-firefox/firefox that referenced this pull request Apr 3, 2026
github-actions Bot pushed a commit to longvatrong111/stylo that referenced this pull request Apr 4, 2026
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.

3 participants