Skip to content

Commit

Permalink
Don't allow new line chars as preceding char for ::first-letter.
Browse files Browse the repository at this point in the history
Because CR and LF new line chars belong to C* unicode category,
but allowed unicode categories as preceding chars are P* and Z*.

But in the context of WhiteSpaceCollapse::kCollapse,
space and new line chars are ignored,
so apply the behavior specified above only Blink::ShouldPreserveBreaks() is true.

Firefox 116.0.2 have this behavior,
but Safari 16.6 shows the same behavior with Chrome of before this change.

This change is per https://drafts.csswg.org/css-pseudo/#first-letter-pattern.

Bug: 1477217
Change-Id: I80aa6a14ecd867f864eb49a12e229e5390dd9a30
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4825211
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Joonghun Park <pjh0718@gmail.com>
Cr-Commit-Position: refs/heads/main@{#1190634}
  • Loading branch information
joonghunpark authored and pull[bot] committed Feb 26, 2024
1 parent 7460920 commit 1091403
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 10 deletions.
54 changes: 45 additions & 9 deletions third_party/blink/renderer/core/dom/first_letter_pseudo_element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,29 +54,57 @@ static inline bool IsPunctuationForFirstLetter(UChar32 c) {
char_category == WTF::unicode::kPunctuation_Other;
}

static inline bool IsSpaceForFirstLetter(UChar c) {
return IsSpaceOrNewline(c) || c == WTF::unicode::kNoBreakSpaceCharacter;
static inline bool IsNewLine(UChar c) {
if (c == 0xA || c == 0xD) {
return true;
}

return false;
}

unsigned FirstLetterPseudoElement::FirstLetterLength(const String& text) {
static inline bool IsSpace(UChar c) {
if (IsNewLine(c)) {
return false;
}

return IsSpaceOrNewline(c);
}

static inline bool IsSpaceForFirstLetter(UChar c, bool preserve_breaks) {
return (RuntimeEnabledFeatures::
CSSFirstLetterNoNewLineAsPrecedingCharEnabled() &&
preserve_breaks
? IsSpace(c)
: IsSpaceOrNewline(c)) ||
c == WTF::unicode::kNoBreakSpaceCharacter;
}

unsigned FirstLetterPseudoElement::FirstLetterLength(const String& text,
bool preserve_breaks) {
unsigned length = 0;
unsigned text_length = text.length();

if (text_length == 0)
return length;

// Account for leading spaces first.
while (length < text_length && IsSpaceForFirstLetter(text[length]))
while (length < text_length &&
IsSpaceForFirstLetter(text[length], preserve_breaks)) {
length++;
}

// Now account for leading punctuation.
while (length < text_length &&
IsPunctuationForFirstLetter(text.CharacterStartingAt(length)))
IsPunctuationForFirstLetter(text.CharacterStartingAt(length))) {
length += LengthOfGraphemeCluster(text, length);
}

// Bail if we didn't find a letter before the end of the text or before a
// space.
if (IsSpaceForFirstLetter(text[length]) || length == text_length)
if (IsSpaceForFirstLetter(text[length], preserve_breaks) ||
IsNewLine(text[length]) || length == text_length) {
return 0;
}

// Account the next character for first letter.
length += LengthOfGraphemeCluster(text, length);
Expand Down Expand Up @@ -153,7 +181,9 @@ LayoutText* FirstLetterPseudoElement::FirstLetterTextLayoutObject(
? To<LayoutTextFragment>(first_letter_text_layout_object)
->CompleteText()
: layout_text->OriginalText();
if (FirstLetterLength(str.Impl()) ||
bool preserve_breaks = ShouldPreserveBreaks(
first_letter_text_layout_object->StyleRef().GetWhiteSpaceCollapse());
if (FirstLetterLength(str.Impl(), preserve_breaks) ||
IsInvalidFirstLetterLayoutObject(first_letter_text_layout_object)) {
break;
}
Expand Down Expand Up @@ -248,7 +278,10 @@ void FirstLetterPseudoElement::UpdateTextFragments() {
String old_text(remaining_text_layout_object_->CompleteText());
DCHECK(old_text.Impl());

unsigned length = FirstLetterPseudoElement::FirstLetterLength(old_text);
bool preserve_breaks = ShouldPreserveBreaks(
remaining_text_layout_object_->StyleRef().GetWhiteSpaceCollapse());
unsigned length =
FirstLetterPseudoElement::FirstLetterLength(old_text, preserve_breaks);
remaining_text_layout_object_->SetTextFragment(
old_text.Impl()->Substring(length, old_text.length()), length,
old_text.length() - length);
Expand Down Expand Up @@ -371,7 +404,10 @@ void FirstLetterPseudoElement::AttachFirstLetterTextLayoutObjects(

// FIXME: This would already have been calculated in firstLetterLayoutObject.
// Can we pass the length through?
unsigned length = FirstLetterPseudoElement::FirstLetterLength(old_text);
bool preserve_breaks = ShouldPreserveBreaks(
first_letter_text->StyleRef().GetWhiteSpaceCollapse());
unsigned length =
FirstLetterPseudoElement::FirstLetterLength(old_text, preserve_breaks);

// In case of inline level content made of punctuation, we use
// first_letter_text length instead of FirstLetterLength.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ class CORE_EXPORT FirstLetterPseudoElement final : public PseudoElement {
void Trace(Visitor*) const override;

static LayoutText* FirstLetterTextLayoutObject(const Element&);
static unsigned FirstLetterLength(const String&);
static unsigned FirstLetterLength(const String&,
bool preserve_breaks = false);

void ClearRemainingTextLayoutObject();
LayoutTextFragment* RemainingTextLayoutObject() const {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -872,6 +872,12 @@
name: "CSSExponentialFunctions",
status: "experimental",
},
{
// Don't allow new line as preceding character for ::first-letter.
// See crbug.com/1477217
name: "CSSFirstLetterNoNewLineAsPrecedingChar",
status: "stable",
},
{
name: "CSSFocusVisible",
status: "stable",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>CSS Test: ::first-letter with preceding new line</title>
<link rel="author" title="Joonghun Park" href="mailto:pjh0718@gmail.com">
<link rel="match" href="reference/first-letter-with-preceding-new-line-ref.html">
<link rel="help" href="https://drafts.csswg.org/css-pseudo/#first-letter-pattern">
<meta name="assert" content="Test checks if preceding new line character is allowed or not">
<style>
body { font-size: 30px; }
#sample {
margin: 20px;
white-space: pre;
}
#sample::first-line {
color: red;
}
#sample::first-letter {
color: green;
border: solid 1px blue;
}
</style>
</head>
<body>
<div id="sample">
The second line.
The third line.
</div>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>CSS Test: ::first-letter with preceding new line</title>
<style>
body { font-size: 30px; }
#sample {
margin: 20px;
white-space: pre;
}
</style>
</head>
<body>
<div id="sample">
The second line.
The third line.
</div>
</body>
</html>

0 comments on commit 1091403

Please sign in to comment.