Changeset 290726 in webkit

Timestamp:

Mar 2, 2022 6:49:22 AM (5 months ago)

Author:

Alan Bujtas

Message:

[RTL] Incorrect alt text position in right to left context
​https://bugs.webkit.org/show_bug.cgi?id=237352
<rdar://problem/89657704>

Reviewed by Simon Fraser.

Source/WebCore:

Take the inline direction into account when computing the alt text location.

• rendering/RenderImage.cpp:

(WebCore::RenderImage::paintReplaced): This is mostly moving things around/modernizing it.

LayoutTests:

• fast/images/alt-text-with-right-to-left-inline-direction-expected-mismatch.html: Added.
• fast/images/alt-text-with-right-to-left-inline-direction.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/images/alt-text-with-right-to-left-inline-direction-expected-mismatch.html (added)
• LayoutTests/fast/images/alt-text-with-right-to-left-inline-direction.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderImage.cpp (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2022-03-02  Alan Bujtas  <zalan@apple.com>
+
+        [RTL] Incorrect alt text position in right to left context
+        https://bugs.webkit.org/show_bug.cgi?id=237352
+        <rdar://problem/89657704>
+
+        Reviewed by Simon Fraser.
+
+        * fast/images/alt-text-with-right-to-left-inline-direction-expected-mismatch.html: Added.
+        * fast/images/alt-text-with-right-to-left-inline-direction.html: Added.
+
 2022-03-02  Carlos Garcia Campos  <cgarcia@igalia.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2022-03-02  Alan Bujtas  <zalan@apple.com>
+
+        [RTL] Incorrect alt text position in right to left context
+        https://bugs.webkit.org/show_bug.cgi?id=237352
+        <rdar://problem/89657704>
+
+        Reviewed by Simon Fraser.
+
+        Take the inline direction into account when computing the alt text location.
+
+        * rendering/RenderImage.cpp:
+        (WebCore::RenderImage::paintReplaced): This is mostly moving things around/modernizing it.
+
 2022-03-02  Carlos Garcia Campos  <cgarcia@igalia.com>
 

trunk/Source/WebCore/rendering/RenderImage.cpp

@@ -498 +498 @@
             LayoutUnit leftBorder = borderLeft();
             LayoutUnit topBorder = borderTop();
-            LayoutUnit leftPad = paddingLeft();
-            LayoutUnit topPad = paddingTop();
+            LayoutUnit leftPadding = paddingLeft();
+            LayoutUnit topPadding = paddingTop();
 
             bool errorPictureDrawn = false;
@@ -523 +523 @@
                 if (centerY < 0)
                     centerY = 0;
-                imageOffset = LayoutSize(leftBorder + leftPad + centerX + missingImageBorderWidth, topBorder + topPad + centerY + missingImageBorderWidth);
+                imageOffset = LayoutSize(leftBorder + leftPadding + centerX + missingImageBorderWidth, topBorder + topPadding + centerY + missingImageBorderWidth);
 
                 context.drawImage(*image, snapRectToDevicePixels(LayoutRect(paintOffset + imageOffset, imageSize), deviceScaleFactor), { imageOrientation() });
@@ -530 +530 @@
 
             if (!m_altText.isEmpty()) {
-                String text = document().displayStringModifiedByEncoding(m_altText);
-                context.setFillColor(style().visitedDependentColorWithColorFilter(CSSPropertyColor));
-                const FontCascade& font = style().fontCascade();
-                const FontMetrics& fontMetrics = font.metricsOfPrimaryFont();
-                LayoutUnit ascent = fontMetrics.ascent();
-                LayoutPoint altTextOffset = paintOffset;
-                altTextOffset.move(leftBorder + leftPad + (paddingWidth / 2) - missingImageBorderWidth, topBorder + topPad + ascent + (paddingHeight / 2) - missingImageBorderWidth);
-
-                // Only draw the alt text if it'll fit within the content box,
-                // and only if it fits above the error image.
-                TextRun textRun = RenderBlock::constructTextRun(text, style());
-                LayoutUnit textWidth { font.width(textRun) };
-                if (errorPictureDrawn) {
-                    if (usableSize.width() >= textWidth && fontMetrics.height() <= imageOffset.height())
-                        context.drawBidiText(font, textRun, altTextOffset);
-                } else if (usableSize.width() >= textWidth && usableSize.height() >= fontMetrics.height())
-                    context.drawBidiText(font, textRun, altTextOffset);
+                auto& font = style().fontCascade();
+                auto& fontMetrics = font.metricsOfPrimaryFont();
+                auto textRun = RenderBlock::constructTextRun(document().displayStringModifiedByEncoding(m_altText), style());
+                auto textWidth = LayoutUnit { font.width(textRun) };
+
+                auto hasRoomForAltText = [&] {
+                    // Only draw the alt text if it'll fit within the content box,
+                    // and only if it fits above the error image.
+                    if (usableSize.width() < textWidth)
+                        return false;
+                    return errorPictureDrawn ? fontMetrics.height() <= imageOffset.height() : usableSize.height() >= fontMetrics.height();
+                };
+                if (hasRoomForAltText()) {
+                    auto altTextLocation = [&]() -> LayoutPoint {
+                        auto contentHorizontalOffset = LayoutUnit { leftBorder + leftPadding + (paddingWidth / 2) - missingImageBorderWidth };
+                        auto contentVerticalOffset = LayoutUnit { topBorder + topPadding + fontMetrics.ascent() + (paddingHeight / 2) - missingImageBorderWidth };
+                        if (!style().isLeftToRightDirection())
+                            contentHorizontalOffset += contentSize.width() - textWidth;
+                        return paintOffset + LayoutPoint { contentHorizontalOffset, contentVerticalOffset };
+                    };
+                    context.setFillColor(style().visitedDependentColorWithColorFilter(CSSPropertyColor));
+                    context.drawBidiText(font, textRun, altTextLocation());
+                }
             }
         }