Skip to content

Conversation

@nicoburns
Copy link
Collaborator

This follows https://www.w3.org/TR/CSS2/text.html#white-space-model. Further work will be required to completely match that spec. In particular, whitespace preceding a newline also ought to be stripped but:

  1. This is harder to implement in the current whitespace collapsing code
  2. This is less consequential as trailing whitespace isn't usually visible.

This fixes a visible 1-space indent at the start of lines following a <br /> tag in Blitz.

@nicoburns nicoburns added the bug Something isn't working label Jan 24, 2025
@nicoburns nicoburns force-pushed the strip-whitespace-following-a-newline branch from b6ef606 to a04ce41 Compare January 24, 2025 22:02
Copy link
Member

@xorgy xorgy left a comment

Choose a reason for hiding this comment

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

This makes sense, as long as the documented purpose of WhiteSpaceCollapse::Collapse is to follow the CSS whitespace processing model.

Copy link
Member

@tomcur tomcur left a comment

Choose a reason for hiding this comment

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

LGTM.

@nicoburns nicoburns added this pull request to the merge queue Jan 29, 2025
Merged via the queue into linebender:main with commit c961a82 Jan 29, 2025
21 checks passed
@nicoburns nicoburns deleted the strip-whitespace-following-a-newline branch January 29, 2025 21:48
github-merge-queue bot pushed a commit that referenced this pull request Jan 31, 2025
Adds `tree_builder` to `TestEnv` to be able to set white space
collapsing mode.

A quick follow-up to #254.
github-merge-queue bot pushed a commit that referenced this pull request Feb 20, 2025
Followup (small bug fix) to
#254 which didn't take inline
boxes into account. That PR was checking whether "the last character is
whitespace" without checking whether the last thing pushed into the
layout was text.

So if the sequence of items pushed to a layout was something like:

- inline box
- whitespace
- inline box
- whitespace
- inline box
- whitespace
- inline box

(an example which would be "status badges" in Github readme's which are
typically images separated by whitespace characters)

Then all but the first "whitespace" would get collapsed as it would be
incorrectly detected as directly adjacent to the preceding whitespace
(even though it shouldn't because there is an inline box in between).

This PR fixes this issue by tracking the kind of the last item pushed.

---------

Signed-off-by: Nico Burns <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants