Skip to content

Commit

Permalink
Align criteria for isAlignedForUnder.
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=270431

Reviewed by Alan Baradlay.

The criteria of isAlignedForUnder is not aligned between RenderStyle::changeAffectsVisualOverflow from and isAlignedForUnder in InlineTextBoxStyle.

* LayoutTests/fast/css3-text/css3-text-decoration/text-underline-position/text-underline-position-auto-computed-underline-offset-crash-expected.txt: Added.
* LayoutTests/fast/css3-text/css3-text-decoration/text-underline-position/text-underline-position-auto-computed-underline-offset-crash.html: Added.
* Source/WebCore/rendering/style/RenderStyle.cpp:
(WebCore::RenderStyle::changeAffectsVisualOverflow const):
* Source/WebCore/style/InlineTextBoxStyle.cpp:
(WebCore::isAlignedForUnder):
* Source/WebCore/style/InlineTextBoxStyle.h:

Canonical link: https://commits.webkit.org/275771@main
  • Loading branch information
lericaa authored and alanbaradlay committed Mar 7, 2024
1 parent 88b3ecb commit e644c80
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 19 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
PASS if no crash or assert.
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<style>
html {
writing-mode: vertical-lr;
}
</style>
<span id=under>PASS</span> if no crash or assert.
<script>
if (window.testRunner)
testRunner.dumpAsText();
document.body.offsetHeight;
under.style.textDecoration = "underline";
under.style.textUnderlinePosition = "auto";
</script>
5 changes: 1 addition & 4 deletions Source/WebCore/rendering/style/RenderStyle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -717,10 +717,7 @@ inline bool RenderStyle::changeAffectsVisualOverflow(const RenderStyle& other) c
|| m_rareInheritedData->textUnderlinePosition != other.m_rareInheritedData->textUnderlinePosition) {
// Underlines are always drawn outside of their textbox bounds when text-underline-position: under;
// is specified. We can take an early out here.
auto isVertialWritingMode = isVerticalWritingMode() || other.isVerticalWritingMode();
auto isAlignedForUnder = textUnderlinePosition() == TextUnderlinePosition::Under || other.textUnderlinePosition() == TextUnderlinePosition::Under
|| (isVertialWritingMode && (textUnderlinePosition() == TextUnderlinePosition::Right || other.textUnderlinePosition() == TextUnderlinePosition::Right || textUnderlinePosition() == TextUnderlinePosition::Left || other.textUnderlinePosition() == TextUnderlinePosition::Left));
if (isAlignedForUnder)
if (isAlignedForUnder(*this) || isAlignedForUnder(other))
return true;
return visualOverflowForDecorations(*this) != visualOverflowForDecorations(other);
}
Expand Down
30 changes: 15 additions & 15 deletions Source/WebCore/style/InlineTextBoxStyle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,21 +42,6 @@ struct UnderlineOffsetArguments {
std::optional<TextUnderlinePositionUnder> textUnderlinePositionUnder { };
};

static bool isAlignedForUnder(const RenderStyle& decoratingBoxStyle)
{
auto underlinePosition = decoratingBoxStyle.textUnderlinePosition();
if (underlinePosition == TextUnderlinePosition::Under)
return true;
if (decoratingBoxStyle.isHorizontalWritingMode())
return false;
if (underlinePosition == TextUnderlinePosition::Left || underlinePosition == TextUnderlinePosition::Right) {
// In vertical typographic modes, the underline is aligned as for under for 'left' and 'right'.
return true;
}
// When left/right support is not enabled.
return underlinePosition == TextUnderlinePosition::Auto && decoratingBoxStyle.textUnderlineOffset().isAuto();
}

static bool isAncestorAndWithinBlock(const RenderInline& ancestor, const RenderObject* child)
{
const RenderObject* object = child;
Expand Down Expand Up @@ -279,6 +264,21 @@ static GlyphOverflow computedVisualOverflowForDecorations(const RenderStyle& lin
return overflowResult;
}

bool isAlignedForUnder(const RenderStyle& decoratingBoxStyle)
{
auto underlinePosition = decoratingBoxStyle.textUnderlinePosition();
if (underlinePosition == TextUnderlinePosition::Under)
return true;
if (decoratingBoxStyle.isHorizontalWritingMode())
return false;
if (underlinePosition == TextUnderlinePosition::Left || underlinePosition == TextUnderlinePosition::Right) {
// In vertical typographic modes, the underline is aligned as for under for 'left' and 'right'.
return true;
}
// When left/right support is not enabled.
return underlinePosition == TextUnderlinePosition::Auto && decoratingBoxStyle.textUnderlineOffset().isAuto();
}

GlyphOverflow visualOverflowForDecorations(const InlineIterator::LineBoxIterator& lineBox, const RenderText& renderer, float textBoxLogicalTop, float textBoxLogicalBottom)
{
auto& style = lineBox->isFirst() ? renderer.firstLineStyle() : renderer.style();
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/style/InlineTextBoxStyle.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ struct TextUnderlinePositionUnder {
GlyphOverflow visualOverflowForDecorations(const RenderStyle&);
GlyphOverflow visualOverflowForDecorations(const RenderStyle&, TextUnderlinePositionUnder);
GlyphOverflow visualOverflowForDecorations(const InlineIterator::LineBoxIterator&, const RenderText&, float textBoxLogicalTop, float textBoxLogicalBottom);
bool isAlignedForUnder(const RenderStyle& decoratingBoxStyle);

float underlineOffsetForTextBoxPainting(const InlineIterator::InlineBox&, const RenderStyle&);
float overlineOffsetForTextBoxPainting(const InlineIterator::InlineBox&, const RenderStyle&);
Expand Down

0 comments on commit e644c80

Please sign in to comment.