Changeset 225736 in webkit


Ignore:
Timestamp:
Dec 10, 2017 7:10:41 PM (6 years ago)
Author:
commit-queue@webkit.org
Message:

Incorrect bounds inside <mover>/<munder> when a stretchy operator is present
​https://bugs.webkit.org/show_bug.cgi?id=179682

Patch by Minsheng Liu <​lambda@liu.ms> on 2017-12-10
Reviewed by Frédéric Wang.

Source/WebCore:

Currently a stretchy operator inside <mover>/<munder>/<munderover> is stretched
during paint() rather than layout(), which leads to both end user confusion
and many unexpected behaviors. This patch rewrites
RenderMathMLUnderOver::stretchHorizontalOperatorsAndLayoutChildren()
to both eliminate the issue and make operator stretching more standard
conforming.

A test is added to check the stretch width of stretchy operators in various
scenarios:
mathml/opentype/munderover-stretch-width.html

A previous test is updated:
mathml/opentype/opentype-stretchy-horizontal.html

  • rendering/mathml/RenderMathMLOperator.cpp:

(WebCore::RenderMathMLOperator::stretchTo):
(WebCore::RenderMathMLOperator::resetStretchSize):
(WebCore::RenderMathMLOperator::paint):

  • rendering/mathml/RenderMathMLOperator.h:

(WebCore::RenderMathMLOperator::setStretchWidthLocked):
(WebCore::RenderMathMLOperator::isStretchWidthLocked const):

  • rendering/mathml/RenderMathMLUnderOver.cpp:

(WebCore::toHorizontalStretchyOperator):
(WebCore::fixLayoutAfterStretch):
(WebCore::RenderMathMLUnderOver::stretchHorizontalOperatorsAndLayoutChildren):

LayoutTests:

Added test case: mathml/opentype/munderover-stretch-width.html

Updated test case: mathml/opentype/opentype-stretchy-horizontal
We update the test file to make sure the stretchy <mo> has zero lspace/rspace.
Expected results for macOS and iOS are included.

  • mathml/opentype/munderover-stretch-width-expected.txt: Added.
  • mathml/opentype/munderover-stretch-width.html: Added.
  • mathml/opentype/opentype-stretchy-horizontal.html:
  • platform/gtk/mathml/opentype/opentype-stretchy-horizontal-expected.txt: Removed.
  • platform/ios/mathml/opentype/opentype-stretchy-horizontal-expected.png:
  • platform/ios/mathml/opentype/opentype-stretchy-horizontal-expected.txt:
  • platform/mac/mathml/opentype/opentype-stretchy-horizontal-expected.png:
  • platform/mac/mathml/opentype/opentype-stretchy-horizontal-expected.txt:
  • platform/win/mathml/opentype/opentype-stretchy-horizontal-expected.txt: Removed.
Location:
trunk
Files:
2 added
2 deleted
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r225729 r225736  
     12017-12-10  Minsheng Liu  <lambda@liu.ms>
     2
     3        Incorrect bounds inside <mover>/<munder> when a stretchy operator is present
     4        https://bugs.webkit.org/show_bug.cgi?id=179682
     5
     6        Reviewed by Frédéric Wang.
     7
     8        Added test case: mathml/opentype/munderover-stretch-width.html
     9
     10        Updated test case: mathml/opentype/opentype-stretchy-horizontal
     11        We update the test file to make sure the stretchy <mo> has zero lspace/rspace.
     12        Expected results for macOS and iOS are included.
     13
     14        * mathml/opentype/munderover-stretch-width-expected.txt: Added.
     15        * mathml/opentype/munderover-stretch-width.html: Added.
     16        * mathml/opentype/opentype-stretchy-horizontal.html:
     17        * platform/gtk/mathml/opentype/opentype-stretchy-horizontal-expected.txt: Removed.
     18        * platform/ios/mathml/opentype/opentype-stretchy-horizontal-expected.png:
     19        * platform/ios/mathml/opentype/opentype-stretchy-horizontal-expected.txt:
     20        * platform/mac/mathml/opentype/opentype-stretchy-horizontal-expected.png:
     21        * platform/mac/mathml/opentype/opentype-stretchy-horizontal-expected.txt:
     22        * platform/win/mathml/opentype/opentype-stretchy-horizontal-expected.txt: Removed.
     23
    1242017-12-09  Darin Adler  <darin@apple.com>
    225

  • trunk/LayoutTests/mathml/opentype/opentype-stretchy-horizontal.html

    r170232 r225736  
    2222      <math>
    2323        <mstyle scriptsizemultiplier="1">
    24           <mover><mo stretchy="true">&#x219C;</mo><mspace width="1em" height="1px"/></mover>
     24          <mover><mo lspace="0" rspace="0" stretchy="true">&#x219C;</mo><mspace width="1em" height="1px"/></mover>
    2525        </mstyle>
    2626      </math>
    … …  
    2929      <math>
    3030        <mstyle scriptsizemultiplier="1">
    31           <mover><mo stretchy="true">&#x219C;</mo><mspace width="2em" height="1px"/></mover>
     31          <mover><mo lspace="0" rspace="0" stretchy="true">&#x219C;</mo><mspace width="2em" height="1px"/></mover>
    3232        </mstyle>
    3333      </math>
    … …  
    3838      <math>
    3939        <mstyle scriptsizemultiplier="1">
    40           <mover><mo stretchy="true">&#x219C;</mo><mspace width="20em" height="1px"/></mover>
     40          <mover><mo lspace="0" rspace="0" stretchy="true">&#x219C;</mo><mspace width="20em" height="1px"/></mover>
    4141        </mstyle>
    4242      </math>

  • trunk/LayoutTests/platform/ios/mathml/opentype/opentype-stretchy-horizontal-expected.txt

    r203171 r225736  
    55    RenderBody {BODY} at (8,16) size 784x123
    66      RenderBlock {P} at (0,0) size 784x25
    7         RenderMathMLMath {math} at (0,4) size 8x22
    8           RenderMathMLRow {mstyle} at (0,0) size 8x22
    9             RenderMathMLUnderOver {mover} at (0,0) size 8x22
    10               RenderMathMLOperator {mo} at (0,1) size 8x21
     7        RenderMathMLMath {math} at (0,4) size 11x22
     8          RenderMathMLRow {mstyle} at (0,0) size 11x22
     9            RenderMathMLUnderOver {mover} at (0,0) size 11x22
     10              RenderMathMLOperator {mo} at (0,1) size 11x21
    1111                RenderBlock (anonymous) at (0,0) size 2x4
    1212                  RenderText {#text} at (0,-3) size 2x0
    1313                    text run at (0,-3) width 2: "\x{219C}"
    14               RenderMathMLSpace {mspace} at (0,0) size 8x1
     14              RenderMathMLSpace {mspace} at (1,0) size 8x1
    1515        RenderText {#text} at (0,0) size 0x0
    1616      RenderBlock {P} at (0,41) size 784x25
    17         RenderMathMLMath {math} at (0,4) size 15x22
    18           RenderMathMLRow {mstyle} at (0,0) size 15x22
    19             RenderMathMLUnderOver {mover} at (0,0) size 15x22
    20               RenderMathMLOperator {mo} at (4,1) size 7x21
     17        RenderMathMLMath {math} at (0,4) size 21x22
     18          RenderMathMLRow {mstyle} at (0,0) size 21x22
     19            RenderMathMLUnderOver {mover} at (0,0) size 21x22
     20              RenderMathMLOperator {mo} at (0,1) size 21x21
    2121                RenderBlock (anonymous) at (0,0) size 2x4
    2222                  RenderText {#text} at (0,-3) size 2x0
    2323                    text run at (0,-3) width 2: "\x{219C}"
    24               RenderMathMLSpace {mspace} at (0,0) size 15x1
     24              RenderMathMLSpace {mspace} at (2,0) size 16x1
    2525        RenderText {#text} at (0,0) size 0x0
    2626      RenderBlock {P} at (0,82) size 784x41
    … …  
    2828          RenderMathMLRow {mstyle} at (0,0) size 150x42
    2929            RenderMathMLUnderOver {mover} at (0,0) size 150x42
    30               RenderMathMLOperator {mo} at (71,1) size 8x41
     30              RenderMathMLOperator {mo} at (0,1) size 150x41
    3131                RenderBlock (anonymous) at (0,0) size 2x4
    3232                  RenderText {#text} at (0,-3) size 2x0

  • trunk/LayoutTests/platform/mac/mathml/opentype/opentype-stretchy-horizontal-expected.txt

    r203171 r225736  
    55    RenderBody {BODY} at (8,16) size 784x121
    66      RenderBlock {P} at (0,0) size 784x24
    7         RenderMathMLMath {math} at (0,3) size 8x22
    8           RenderMathMLRow {mstyle} at (0,0) size 8x22
    9             RenderMathMLUnderOver {mover} at (0,0) size 8x22
    10               RenderMathMLOperator {mo} at (0,1) size 8x21
     7        RenderMathMLMath {math} at (0,3) size 11x22
     8          RenderMathMLRow {mstyle} at (0,0) size 11x22
     9            RenderMathMLUnderOver {mover} at (0,0) size 11x22
     10              RenderMathMLOperator {mo} at (0,1) size 11x21
    1111                RenderBlock (anonymous) at (0,0) size 2x4
    1212                  RenderText {#text} at (0,-3) size 2x0
    1313                    text run at (0,-3) width 2: "\x{219C}"
    14               RenderMathMLSpace {mspace} at (0,0) size 8x1
     14              RenderMathMLSpace {mspace} at (1,0) size 8x1
    1515        RenderText {#text} at (0,0) size 0x0
    1616      RenderBlock {P} at (0,40) size 784x24
    17         RenderMathMLMath {math} at (0,3) size 15x22
    18           RenderMathMLRow {mstyle} at (0,0) size 15x22
    19             RenderMathMLUnderOver {mover} at (0,0) size 15x22
    20               RenderMathMLOperator {mo} at (4,1) size 7x21
     17        RenderMathMLMath {math} at (0,3) size 21x22
     18          RenderMathMLRow {mstyle} at (0,0) size 21x22
     19            RenderMathMLUnderOver {mover} at (0,0) size 21x22
     20              RenderMathMLOperator {mo} at (0,1) size 21x21
    2121                RenderBlock (anonymous) at (0,0) size 2x4
    2222                  RenderText {#text} at (0,-3) size 2x0
    2323                    text run at (0,-3) width 2: "\x{219C}"
    24               RenderMathMLSpace {mspace} at (0,0) size 15x1
     24              RenderMathMLSpace {mspace} at (2,0) size 16x1
    2525        RenderText {#text} at (0,0) size 0x0
    2626      RenderBlock {P} at (0,80) size 784x41
    … …  
    2828          RenderMathMLRow {mstyle} at (0,0) size 150x42
    2929            RenderMathMLUnderOver {mover} at (0,0) size 150x42
    30               RenderMathMLOperator {mo} at (71,1) size 8x41
     30              RenderMathMLOperator {mo} at (0,1) size 150x41
    3131                RenderBlock (anonymous) at (0,0) size 2x4
    3232                  RenderText {#text} at (0,-3) size 2x0

  • trunk/Source/WebCore/ChangeLog

    r225731 r225736  
     12017-12-10  Minsheng Liu  <lambda@liu.ms>
     2
     3        Incorrect bounds inside <mover>/<munder> when a stretchy operator is present
     4        https://bugs.webkit.org/show_bug.cgi?id=179682
     5
     6        Reviewed by Frédéric Wang.
     7
     8        Currently a stretchy operator inside <mover>/<munder>/<munderover> is stretched
     9        during paint() rather than layout(), which leads to both end user confusion
     10        and many unexpected behaviors. This patch rewrites
     11        RenderMathMLUnderOver::stretchHorizontalOperatorsAndLayoutChildren()
     12        to both eliminate the issue and make operator stretching more standard
     13        conforming.
     14
     15        A test is added to check the stretch width of stretchy operators in various
     16        scenarios:
     17        mathml/opentype/munderover-stretch-width.html
     18
     19        A previous test is updated:
     20        mathml/opentype/opentype-stretchy-horizontal.html
     21
     22        * rendering/mathml/RenderMathMLOperator.cpp:
     23        (WebCore::RenderMathMLOperator::stretchTo):
     24        (WebCore::RenderMathMLOperator::resetStretchSize):
     25        (WebCore::RenderMathMLOperator::paint):
     26        * rendering/mathml/RenderMathMLOperator.h:
     27        (WebCore::RenderMathMLOperator::setStretchWidthLocked):
     28        (WebCore::RenderMathMLOperator::isStretchWidthLocked const):
     29        * rendering/mathml/RenderMathMLUnderOver.cpp:
     30        (WebCore::toHorizontalStretchyOperator):
     31        (WebCore::fixLayoutAfterStretch):
     32        (WebCore::RenderMathMLUnderOver::stretchHorizontalOperatorsAndLayoutChildren):
     33
    1342017-12-10  Yusuke Suzuki  <utatane.tea@gmail.com>
    235

  • trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp

    r224537 r225736  
    123123    ASSERT(isStretchy());
    124124    ASSERT(isVertical());
     125    ASSERT(!isStretchWidthLocked());
    125126
    126127    if (!isVertical() || (heightAboveBaseline == m_stretchHeightAboveBaseline && depthBelowBaseline == m_stretchDepthBelowBaseline))
    … …  
    163164    ASSERT(isStretchy());
    164165    ASSERT(!isVertical());
     166    ASSERT(!isStretchWidthLocked());
    165167
    166168    if (isVertical() || m_stretchWidth == width)
    … …  
    170172    m_mathOperator.stretchTo(style(), width);
    171173
     174    setLogicalWidth(leadingSpace() + width + trailingSpace());
    172175    setLogicalHeight(m_mathOperator.ascent() + m_mathOperator.descent());
    173176}
    … …  
    175178void RenderMathMLOperator::resetStretchSize()
    176179{
     180    ASSERT(!isStretchWidthLocked());
     181   
    177182    if (isVertical()) {
    178183        m_stretchHeightAboveBaseline = 0;
    … …  
    307312    operatorTopLeft.move(style().isLeftToRightDirection() ? leadingSpace() : trailingSpace(), 0);
    308313
    309     // Center horizontal operators.
    310     if (!isVertical())
    311         operatorTopLeft.move(-(m_mathOperator.width() - width()) / 2, 0);
    312 
    313314    m_mathOperator.paint(style(), info, operatorTopLeft);
    314315}

  • trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.h

    r224537 r225736  
    4747    LayoutUnit stretchSize() const { return isVertical() ? m_stretchHeightAboveBaseline + m_stretchDepthBelowBaseline : m_stretchWidth; }
    4848    void resetStretchSize();
     49    void setStretchWidthLocked(bool stretchWidthLocked) { m_isStretchWidthLocked = stretchWidthLocked; }
     50    bool isStretchWidthLocked() const { return m_isStretchWidthLocked; }
    4951
    5052    virtual bool hasOperatorFlag(MathMLOperatorDictionary::Flag) const;
    … …  
    8688    LayoutUnit m_stretchDepthBelowBaseline { 0 };
    8789    LayoutUnit m_stretchWidth;
     90    bool m_isStretchWidthLocked { false };
    8891
    8992    MathOperator m_mathOperator;

  • trunk/Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp

    r225475 r225736  
    5151}
    5252
     53static RenderMathMLOperator* toHorizontalStretchyOperator(RenderBox* box)
     54{
     55    if (is<RenderMathMLBlock>(box)) {
     56        if (auto renderOperator = downcast<RenderMathMLBlock>(*box).unembellishedOperator()) {
     57            if (renderOperator->isStretchy() && !renderOperator->isVertical() && !renderOperator->isStretchWidthLocked())
     58                return renderOperator;
     59        }
     60    }
     61    return nullptr;
     62}
     63   
     64static void fixLayoutAfterStretch(RenderBox* ancestor, RenderMathMLOperator* stretchyOperator)
     65{
     66    stretchyOperator->setStretchWidthLocked(true);
     67    stretchyOperator->setNeedsLayout();
     68    ancestor->layoutIfNeeded();
     69    stretchyOperator->setStretchWidthLocked(false);
     70}
     71
    5372void RenderMathMLUnderOver::stretchHorizontalOperatorsAndLayoutChildren()
    5473{
     74    ASSERT(isValid());
     75    ASSERT(needsLayout());
     76   
     77    Vector<RenderBox*, 3> embellishedOperators;
     78    Vector<RenderMathMLOperator*, 3> stretchyOperators;
     79    bool isAllStretchyOperators = true;
    5580    LayoutUnit stretchWidth = 0;
    56     Vector<RenderMathMLOperator*, 2> renderOperators;
    57 
     81   
    5882    for (auto* child = firstChildBox(); child; child = child->nextSiblingBox()) {
    59         if (child->needsLayout()) {
    60             if (is<RenderMathMLBlock>(child)) {
    61                 if (auto renderOperator = downcast<RenderMathMLBlock>(*child).unembellishedOperator()) {
    62                     if (renderOperator->isStretchy() && !renderOperator->isVertical()) {
    63                         renderOperator->resetStretchSize();
    64                         renderOperators.append(renderOperator);
    65                     }
    66                 }
    67             }
    68 
    69             child->layout();
    70         }
    71 
    72         // Skipping the embellished op does not work for nested structures like
    73         // <munder><mover><mo>_</mo>...</mover> <mo>_</mo></munder>.
    74         stretchWidth = std::max(stretchWidth, child->logicalWidth());
    75     }
    76 
    77     // Set the sizes of (possibly embellished) stretchy operator children.
    78     for (auto& renderOperator : renderOperators)
    79         renderOperator->stretchTo(stretchWidth);
     83        if (auto* stretchyOperator = toHorizontalStretchyOperator(child)) {
     84            embellishedOperators.append(child);
     85            stretchyOperators.append(stretchyOperator);
     86            stretchyOperator->resetStretchSize();
     87            fixLayoutAfterStretch(child, stretchyOperator);
     88        } else {
     89            isAllStretchyOperators = false;
     90            child->layoutIfNeeded();
     91            stretchWidth = std::max(stretchWidth, child->logicalWidth());
     92        }
     93    }
     94   
     95    if (isAllStretchyOperators) {
     96        for (auto* embellishedOperator : embellishedOperators)
     97            stretchWidth = std::max(stretchWidth, embellishedOperator->logicalWidth());
     98    }
     99   
     100    for (size_t i = 0; i < embellishedOperators.size(); i++) {
     101        stretchyOperators[i]->stretchTo(stretchWidth);
     102        fixLayoutAfterStretch(embellishedOperators[i], stretchyOperators[i]);
     103    }
    80104}
    81105
Note: See TracChangeset for help on using the changeset viewer.