Skip to content

Conversation

@RichardTjokroutomo
Copy link

@RichardTjokroutomo RichardTjokroutomo commented Nov 18, 2025

Enable text-overflow for Servo. Companion PR: #40526

Copy link
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.

As said in the Servo review, since it's not handling strings or 2 values, this should alter the parser to reject these cases for Servo.

Alternatively, you can put this property behind a flag, and then in Servo make it an experimental web platform feature flag. Then it won't be enabled by default, but it will be used in tests.

@RichardTjokroutomo
Copy link
Author

@Loirooriol I noticed earlier today that a PR on Stylo has been merged (#271), so I tried to rebase. However, I got some compilation errors.

Copy link
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.

Since changing the struct affects things more than what I expected, maybe just do something like

        #[cfg(feature = "servo")]
        if matches!(first, TextOverflowSide::String(_)) {
            return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
        }
        #[cfg(feature = "gecko")]
        if let Ok(second) = input.try_parse(|input| TextOverflowSide::parse(context, input)) {
            return Ok(Self {
                first,
                second,
                sides_are_logical: false,
            });
        }
        Ok(Self {
            first: TextOverflowSide::Clip,
            second: first,
            sides_are_logical: true,
        })

Or add support for strings as Nico says in servo/servo#40526 (comment)

Signed-off-by: Richard Tjokroutomo <[email protected]>
Co-authored-by: Oriol Brufau <[email protected]>
@RichardTjokroutomo RichardTjokroutomo force-pushed the txt-overflow branch 2 times, most recently from 6bdf687 to cb461f6 Compare November 27, 2025 21:51
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.

2 participants