Changeset 270839 in webkit

Timestamp:

Dec 15, 2020, 7:07:04 AM (5 years ago)

Author:

Alan Bujtas

Message:

REGRESSION: apple.com sign-in page footer does not wrap, "Site Map" option renders offscreen.
​https://bugs.webkit.org/show_bug.cgi?id=219880
<rdar://problem/72128221>

Reviewed by Antti Koivisto.

Source/WebCore:

Do not check the inline-block's style for wrapping. It's the parent's style that drives the content wrapping.

E.g. <div style="white-space: nowrap">some text<div style="display: inline-block; white-space: pre-wrap"></div></div>.
While the inline-block has pre-wrap which allows wrapping, the content lives in a nowrap context.

Test: fast/inline/inline-block-no-wrap.html

• layout/inlineformatting/InlineContentBreaker.cpp:

(WebCore::Layout::InlineContentBreaker::processOverflowingContent const):
(WebCore::Layout::lastContentRunIndex): Deleted.
(WebCore::Layout::InlineContentBreaker::isContentWrappingAllowed const): Deleted.

• layout/inlineformatting/InlineContentBreaker.h:

LayoutTests:

• fast/inline/inline-block-no-wrap-expected.html: Added.
• fast/inline/inline-block-no-wrap.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/inline/inline-block-no-wrap-expected.html (added)
• LayoutTests/fast/inline/inline-block-no-wrap.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/layout/inlineformatting/InlineContentBreaker.cpp (modified) (3 diffs)
• Source/WebCore/layout/inlineformatting/InlineContentBreaker.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2020-12-15  Zalan Bujtas  <zalan@apple.com>
+
+        REGRESSION: apple.com sign-in page footer does not wrap, "Site Map" option renders offscreen.
+        https://bugs.webkit.org/show_bug.cgi?id=219880
+        <rdar://problem/72128221>
+
+        Reviewed by Antti Koivisto.
+
+        * fast/inline/inline-block-no-wrap-expected.html: Added.
+        * fast/inline/inline-block-no-wrap.html: Added.
+
 2020-12-15  Martin Robinson  <mrobinson@webkit.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-12-15  Zalan Bujtas  <zalan@apple.com>
+
+        REGRESSION: apple.com sign-in page footer does not wrap, "Site Map" option renders offscreen.
+        https://bugs.webkit.org/show_bug.cgi?id=219880
+        <rdar://problem/72128221>
+
+        Reviewed by Antti Koivisto.
+
+        Do not check the inline-block's style for wrapping. It's the parent's style that drives the content wrapping.
+
+        E.g. <div style="white-space: nowrap">some text<div style="display: inline-block; white-space: pre-wrap"></div></div>.
+        While the inline-block has pre-wrap which allows wrapping, the content lives in a nowrap context.
+
+        Test: fast/inline/inline-block-no-wrap.html
+
+        * layout/inlineformatting/InlineContentBreaker.cpp:
+        (WebCore::Layout::InlineContentBreaker::processOverflowingContent const):
+        (WebCore::Layout::lastContentRunIndex): Deleted.
+        (WebCore::Layout::InlineContentBreaker::isContentWrappingAllowed const): Deleted.
+        * layout/inlineformatting/InlineContentBreaker.h:
+
 2020-12-15  Martin Robinson  <mrobinson@webkit.org>
 

trunk/Source/WebCore/layout/inlineformatting/InlineContentBreaker.cpp

@@ -88 +88 @@
 }
 
-static inline Optional<size_t> lastContentRunIndex(const InlineContentBreaker::ContinuousContent& continuousContent)
-{
-    auto& runs = continuousContent.runs();
-    for (auto index = runs.size(); index--;) {
-        if (runs[index].inlineItem.isText() || runs[index].inlineItem.isBox())
-            return index;
-    }
-    return { };
-}
-
 static inline bool isWrappingAllowed(const RenderStyle& style)
 {
@@ -114 +104 @@
     auto lastItemIndex = runList.size() - 1;
     return isWrappingAllowed(runList[lastItemIndex].inlineItem.style()) ? makeOptional(lastItemIndex) : WTF::nullopt;
-}
-
-bool InlineContentBreaker::isContentWrappingAllowed(const ContinuousContent& continuousContent) const
-{
-    // Use the last inline item with content (where we would be wrapping) to decide if content wrapping is allowed.
-    auto& continuousRuns = continuousContent.runs();
-    auto runIndex = lastContentRunIndex(continuousContent).valueOr(continuousRuns.size() - 1);
-    return isWrappingAllowed(continuousRuns[runIndex].inlineItem.style());
 }
 
@@ -236 +218 @@
         return { Result::Action::Keep, IsEndOfLine::No };
     }
-    // Now either wrap here or at an earlier position, or not wrap at all.
-    if (isContentWrappingAllowed(continuousContent))
+    // Now either wrap this content over to the next line or revert back to an earlier wrapping opportunity, or not wrap at all.
+    auto shouldWrapThisContentToNextLine = [&] {
+        // The individual runs in this continuous content don't break, let's check if we are allowed to wrap this content to next line (e.g. pre would prevent us from wrapping).
+        auto& runs = continuousContent.runs();
+        auto& lastInlineItem = runs.last().inlineItem;
+        // Parent style drives the wrapping behavior here.
+        // e.g. <div style="white-space: nowrap">some text<div style="display: inline-block; white-space: pre-wrap"></div></div>.
+        // While the inline-block has pre-wrap which allows wrapping, the content lives in a nowrap context.
+        if (lastInlineItem.isBox() || lastInlineItem.isInlineBoxStart() || lastInlineItem.isInlineBoxStart())
+            return isWrappingAllowed(lastInlineItem.layoutBox().parent().style());
+        if (lastInlineItem.isText()) {
+            if (runs.size() == 1) {
+                // Fast path for the most common case of an individual text item.
+                return isWrappingAllowed(lastInlineItem.layoutBox().style());
+            }
+            for (auto& run : WTF::makeReversedRange(runs)) {
+                auto& inlineItem = run.inlineItem;
+                if (inlineItem.isInlineBoxStart() || inlineItem.isInlineBoxStart())
+                    return isWrappingAllowed(inlineItem.layoutBox().parent().style());
+                ASSERT(!inlineItem.isBox());
+            }
+            // This must be a set of individual text runs. We could just check the last item.
+            return isWrappingAllowed(lastInlineItem.layoutBox().style());
+        }
+        ASSERT_NOT_REACHED();
+        return true;
+    };
+    if (shouldWrapThisContentToNextLine())
         return { Result::Action::Wrap, IsEndOfLine::Yes };
     if (m_hasWrapOpportunityAtPreviousPosition)

trunk/Source/WebCore/layout/inlineformatting/InlineContentBreaker.h

@@ -129 +129 @@
     WordBreakRule wordBreakBehavior(const RenderStyle&) const;
     bool shouldKeepEndOfLineWhitespace(const ContinuousContent&) const;
-    bool isContentWrappingAllowed(const ContinuousContent&) const;
 
     bool n_hyphenationIsDisabled { false };