Diff
Modified: trunk/LayoutTests/ChangeLog (226286 => 226287)
--- trunk/LayoutTests/ChangeLog 2017-12-23 00:00:42 UTC (rev 226286)
+++ trunk/LayoutTests/ChangeLog 2017-12-23 16:05:41 UTC (rev 226287)
@@ -1,3 +1,18 @@
+2017-12-23 Minsheng Liu <[email protected]>
+
+ Add comments and improve code styles for RenderMathMLUnderOver::stretchHorizontalOperatorsAndLayoutChildren() and related functions
+ https://bugs.webkit.org/show_bug.cgi?id=180923
+
+ Reviewed by Frédéric Wang.
+
+ Add one more case for the test "mathml/opentype/munderover-stretch-width.html"
+ to handle the corner case where all components of <munderover>/<munder>/<mover> are stretchy.
+
+ Since there is no behavior change, no new test is required.
+
+ * mathml/opentype/munderover-stretch-width-expected.txt:
+ * mathml/opentype/munderover-stretch-width.html:
+
2017-12-22 Michael Catanzaro <[email protected]>
Unreviewed GTK layout test gardening
Modified: trunk/LayoutTests/mathml/opentype/munderover-stretch-width-expected.txt (226286 => 226287)
--- trunk/LayoutTests/mathml/opentype/munderover-stretch-width-expected.txt 2017-12-23 00:00:42 UTC (rev 226286)
+++ trunk/LayoutTests/mathml/opentype/munderover-stretch-width-expected.txt 2017-12-23 16:05:41 UTC (rev 226287)
@@ -23,7 +23,11 @@
This test passes if you see the black thing has the same width as the red bar.
↜
+This test passes if you see both the red and blue things have the same width as the black bar.
+↜
+↜
+
PASS simple stretchy over
PASS simple stretchy under
PASS embellished op over with wide base
@@ -32,4 +36,5 @@
PASS nested embellished op
PASS nested non-munderover embellished op
PASS simple stretchy under should equal over
+PASS all stretchy embellished op
Modified: trunk/LayoutTests/mathml/opentype/munderover-stretch-width.html (226286 => 226287)
--- trunk/LayoutTests/mathml/opentype/munderover-stretch-width.html 2017-12-23 00:00:42 UTC (rev 226286)
+++ trunk/LayoutTests/mathml/opentype/munderover-stretch-width.html 2017-12-23 16:05:41 UTC (rev 226287)
@@ -50,6 +50,7 @@
runCase(6, 'nested embellished op', 'red', 'op');
runCase(7, 'nested non-munderover embellished op', 'red', 'op');
runCase(8, 'simple stretchy under should equal over', 'red', 'op');
+ runCase(9, 'all stretchy embellished op', 'red', 'blue', 'bar');
done();
}
</script>
@@ -140,5 +141,16 @@
<mspace id="8-red" width="200px" height="10px" depth="10px" mathbackground="red"></mspace>
</munderover>
</math>
+
+ <p>This test passes if you see both the red and blue things have the same width as the black bar.</p>
+ <math display="block">
+ <mover>
+ <mo id="9-blue" lspace="0px" rspace="0px" stretchy="true" mathbackground="blue">↜</mo>
+ <munder>
+ <mo id="9-red" lspace="0px" rspace="0px" stretchy="true" mathbackground="red">↜</mo>
+ <mspace id="9-bar" width="200px" height="10px" depth="10px" mathbackground="black"></mspace>
+ </munder>
+ </mover>
+ </math>
</body>
</html>
Modified: trunk/Source/WebCore/ChangeLog (226286 => 226287)
--- trunk/Source/WebCore/ChangeLog 2017-12-23 00:00:42 UTC (rev 226286)
+++ trunk/Source/WebCore/ChangeLog 2017-12-23 16:05:41 UTC (rev 226287)
@@ -1,3 +1,47 @@
+2017-12-23 Minsheng Liu <[email protected]>
+
+ Add comments and improve code styles for RenderMathMLUnderOver::stretchHorizontalOperatorsAndLayoutChildren() and related functions
+ https://bugs.webkit.org/show_bug.cgi?id=180923
+
+ Reviewed by Frédéric Wang.
+
+ The patch improves the code for RenderMathMLUnderOver::stretchHorizontalOperatorsAndLayoutChildren()
+ and related function, incorporating the following suggestions from bug 179682:
+
+ - Remove several lines of trailing spaces.
+ - Change several pointers to references.
+ - Rewrite horizontalStretchyOperator() (formerly toHorizontalStretchyOperator())
+ to make it more conforming to WebKit's coding style.
+ - Make unembellishedOperator() a const method.
+ - Add comments for stretchHorizontalOperatorsAndLayoutChildren().
+ - Eliminate an unnecessary call to fixLayoutAfterStretch() in stretchHorizontalOperatorsAndLayoutChildren().
+
+ Add one more case for the test "mathml/opentype/munderover-stretch-width.html"
+ to handle the corner case where all components of <munderover>/<munder>/<mover> are stretchy.
+
+ Since there is no behavior change, no new test is required.
+
+ * rendering/mathml/RenderMathMLBlock.h:
+ (WebCore::RenderMathMLBlock::unembellishedOperator const):
+ (WebCore::RenderMathMLBlock::unembellishedOperator): Deleted.
+ * rendering/mathml/RenderMathMLFraction.cpp:
+ (WebCore::RenderMathMLFraction::unembellishedOperator const):
+ (WebCore::RenderMathMLFraction::unembellishedOperator): Deleted.
+ * rendering/mathml/RenderMathMLFraction.h:
+ * rendering/mathml/RenderMathMLOperator.cpp:
+ (WebCore::RenderMathMLOperator::resetStretchSize):
+ * rendering/mathml/RenderMathMLOperator.h:
+ * rendering/mathml/RenderMathMLScripts.cpp:
+ (WebCore::RenderMathMLScripts::unembellishedOperator const):
+ (WebCore::RenderMathMLScripts::unembellishedOperator): Deleted.
+ * rendering/mathml/RenderMathMLScripts.h:
+ * rendering/mathml/RenderMathMLUnderOver.cpp:
+ (WebCore::horizontalStretchyOperator):
+ (WebCore::fixLayoutAfterStretch):
+ (WebCore::RenderMathMLUnderOver::stretchHorizontalOperatorsAndLayoutChildren):
+ (WebCore::RenderMathMLUnderOver::layoutBlock):
+ (WebCore::toHorizontalStretchyOperator): Deleted.
+
2017-12-22 Wenson Hsieh <[email protected]>
Unreviewed, try to fix the build on recent SDKs after r226274.
Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLBlock.h (226286 => 226287)
--- trunk/Source/WebCore/rendering/mathml/RenderMathMLBlock.h 2017-12-23 00:00:42 UTC (rev 226286)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLBlock.h 2017-12-23 16:05:41 UTC (rev 226287)
@@ -57,7 +57,7 @@
// embellished operator, and omits any embellishments.
// FIXME: We don't yet handle all the cases in the MathML spec. See
// https://bugs.webkit.org/show_bug.cgi?id=78617.
- virtual RenderMathMLOperator* unembellishedOperator() { return 0; }
+ virtual RenderMathMLOperator* unembellishedOperator() const { return 0; }
int baselinePosition(FontBaseline, bool firstLine, LineDirectionMode, LinePositionMode = PositionOnContainingLine) const override;
Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLFraction.cpp (226286 => 226287)
--- trunk/Source/WebCore/rendering/mathml/RenderMathMLFraction.cpp 2017-12-23 00:00:42 UTC (rev 226286)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLFraction.cpp 2017-12-23 16:05:41 UTC (rev 226287)
@@ -152,7 +152,7 @@
return parameters;
}
-RenderMathMLOperator* RenderMathMLFraction::unembellishedOperator()
+RenderMathMLOperator* RenderMathMLFraction::unembellishedOperator() const
{
if (!isValid() || !is<RenderMathMLBlock>(numerator()))
return nullptr;
Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLFraction.h (226286 => 226287)
--- trunk/Source/WebCore/rendering/mathml/RenderMathMLFraction.h 2017-12-23 00:00:42 UTC (rev 226286)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLFraction.h 2017-12-23 16:05:41 UTC (rev 226287)
@@ -53,7 +53,7 @@
void layoutBlock(bool relayoutChildren, LayoutUnit pageLogicalHeight = 0) final;
std::optional<int> firstLineBaseline() const final;
void paint(PaintInfo&, const LayoutPoint&) final;
- RenderMathMLOperator* unembellishedOperator() final;
+ RenderMathMLOperator* unembellishedOperator() const final;
MathMLFractionElement& element() const { return static_cast<MathMLFractionElement&>(nodeForNonAnonymous()); }
Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp (226286 => 226287)
--- trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp 2017-12-23 00:00:42 UTC (rev 226286)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp 2017-12-23 16:05:41 UTC (rev 226287)
@@ -178,7 +178,7 @@
void RenderMathMLOperator::resetStretchSize()
{
ASSERT(!isStretchWidthLocked());
-
+
if (isVertical()) {
m_stretchHeightAboveBaseline = 0;
m_stretchDepthBelowBaseline = 0;
Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.h (226286 => 226287)
--- trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.h 2017-12-23 00:00:42 UTC (rev 226286)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.h 2017-12-23 16:05:41 UTC (rev 226287)
@@ -80,7 +80,7 @@
bool isInvisibleOperator() const;
std::optional<int> firstLineBaseline() const final;
- RenderMathMLOperator* unembellishedOperator() final { return this; }
+ RenderMathMLOperator* unembellishedOperator() const final { return const_cast<RenderMathMLOperator*>(this); }
LayoutUnit verticalStretchedOperatorShift() const;
Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLScripts.cpp (226286 => 226287)
--- trunk/Source/WebCore/rendering/mathml/RenderMathMLScripts.cpp 2017-12-23 00:00:42 UTC (rev 226286)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLScripts.cpp 2017-12-23 16:05:41 UTC (rev 226287)
@@ -59,7 +59,7 @@
return element().scriptType();
}
-RenderMathMLOperator* RenderMathMLScripts::unembellishedOperator()
+RenderMathMLOperator* RenderMathMLScripts::unembellishedOperator() const
{
auto base = firstChildBox();
if (!is<RenderMathMLBlock>(base))
Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLScripts.h (226286 => 226287)
--- trunk/Source/WebCore/rendering/mathml/RenderMathMLScripts.h 2017-12-23 00:00:42 UTC (rev 226286)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLScripts.h 2017-12-23 16:05:41 UTC (rev 226287)
@@ -42,7 +42,7 @@
WTF_MAKE_ISO_ALLOCATED(RenderMathMLScripts);
public:
RenderMathMLScripts(MathMLScriptsElement&, RenderStyle&&);
- RenderMathMLOperator* unembellishedOperator() final;
+ RenderMathMLOperator* unembellishedOperator() const final;
protected:
bool isRenderMathMLScripts() const override { return true; }
Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp (226286 => 226287)
--- trunk/Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp 2017-12-23 00:00:42 UTC (rev 226286)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp 2017-12-23 16:05:41 UTC (rev 226287)
@@ -50,23 +50,27 @@
return static_cast<MathMLUnderOverElement&>(nodeForNonAnonymous());
}
-static RenderMathMLOperator* toHorizontalStretchyOperator(RenderBox* box)
+static RenderMathMLOperator* horizontalStretchyOperator(const RenderBox& box)
{
- if (is<RenderMathMLBlock>(box)) {
- if (auto renderOperator = downcast<RenderMathMLBlock>(*box).unembellishedOperator()) {
- if (renderOperator->isStretchy() && !renderOperator->isVertical() && !renderOperator->isStretchWidthLocked())
- return renderOperator;
- }
- }
- return nullptr;
+ if (!is<RenderMathMLBlock>(box))
+ return nullptr;
+
+ auto* renderOperator = downcast<RenderMathMLBlock>(box).unembellishedOperator();
+ if (!renderOperator)
+ return nullptr;
+
+ if (!renderOperator->isStretchy() || renderOperator->isVertical() || renderOperator->isStretchWidthLocked())
+ return nullptr;
+
+ return renderOperator;
}
-
-static void fixLayoutAfterStretch(RenderBox* ancestor, RenderMathMLOperator* stretchyOperator)
+
+static void fixLayoutAfterStretch(RenderBox& ancestor, RenderMathMLOperator& stretchyOperator)
{
- stretchyOperator->setStretchWidthLocked(true);
- stretchyOperator->setNeedsLayout();
- ancestor->layoutIfNeeded();
- stretchyOperator->setStretchWidthLocked(false);
+ stretchyOperator.setStretchWidthLocked(true);
+ stretchyOperator.setNeedsLayout();
+ ancestor.layoutIfNeeded();
+ stretchyOperator.setStretchWidthLocked(false);
}
void RenderMathMLUnderOver::stretchHorizontalOperatorsAndLayoutChildren()
@@ -73,18 +77,29 @@
{
ASSERT(isValid());
ASSERT(needsLayout());
+
+ // We apply horizontal stretchy rules from the MathML spec (sections 3.2.5.8.3 and 3.2.5.8.4), which
+ // can be roughly summarized as "stretching opersators to the maximum widths of all children" and
+ // minor variations of that algorithm do not affect the result. However, the spec is a bit ambiguous
+ // for embellished operators (section 3.2.5.7.3) and different approaches can lead to significant
+ // stretch size differences. We made the following decisions:
+ // - The unstretched size is the embellished operator width with the <mo> at the core unstretched.
+ // - In general, the target size is just the maximum widths of non-stretchy children because the
+ // embellishments could make widths significantly larger.
+ // - In the edge case when all operators of stretchy, we follow the specification and take the
+ // maximum of all unstretched sizes.
+ // - The <mo> at the core is stretched to cover the target size, even if the embellished operator
+ // might become much wider.
Vector<RenderBox*, 3> embellishedOperators;
Vector<RenderMathMLOperator*, 3> stretchyOperators;
bool isAllStretchyOperators = true;
LayoutUnit stretchWidth = 0;
-
+
for (auto* child = firstChildBox(); child; child = child->nextSiblingBox()) {
- if (auto* stretchyOperator = toHorizontalStretchyOperator(child)) {
+ if (auto* stretchyOperator = horizontalStretchyOperator(*child)) {
embellishedOperators.append(child);
stretchyOperators.append(stretchyOperator);
- stretchyOperator->resetStretchSize();
- fixLayoutAfterStretch(child, stretchyOperator);
} else {
isAllStretchyOperators = false;
child->layoutIfNeeded();
@@ -91,15 +106,18 @@
stretchWidth = std::max(stretchWidth, child->logicalWidth());
}
}
-
+
if (isAllStretchyOperators) {
- for (auto* embellishedOperator : embellishedOperators)
- stretchWidth = std::max(stretchWidth, embellishedOperator->logicalWidth());
+ for (size_t i = 0; i < embellishedOperators.size(); i++) {
+ stretchyOperators[i]->resetStretchSize();
+ fixLayoutAfterStretch(*embellishedOperators[i], *stretchyOperators[i]);
+ stretchWidth = std::max(stretchWidth, embellishedOperators[i]->logicalWidth());
+ }
}
-
+
for (size_t i = 0; i < embellishedOperators.size(); i++) {
stretchyOperators[i]->stretchTo(stretchWidth);
- fixLayoutAfterStretch(embellishedOperators[i], stretchyOperators[i]);
+ fixLayoutAfterStretch(*embellishedOperators[i], *stretchyOperators[i]);
}
}
@@ -290,7 +308,7 @@
ASSERT(!base().needsLayout());
ASSERT(scriptType() == ScriptType::Over || !under().needsLayout());
ASSERT(scriptType() == ScriptType::Under || !over().needsLayout());
-
+
LayoutUnit logicalWidth = base().logicalWidth();
if (scriptType() == ScriptType::Under || scriptType() == ScriptType::UnderOver)
logicalWidth = std::max(logicalWidth, under().logicalWidth());
