Bug 156910 – RenderMathMLOperator: add getBaseGlyph and avoid passing m_textContent as a parameter

• Status: RESOLVED
• Resolution: FIXED
• Component: MathML

[Frédéric Wang (:fredw)]

Splitting another first step from bug 152244.

The RenderStyle parameter is not really needed yet until we move getBaseGlyph code, but I'm adding it for consistency with the MathOperator class.

[Frédéric Wang (:fredw)]

Created attachment 277058 [details]
Patch

[Manuel Rego Casasnovas]

Comment on attachment 277058 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=277058&action=review

LGTM. Nice refactoring.

> Source/WebCore/rendering/mathml/RenderMathMLOperator.h:146
> +    StretchyData getDisplayStyleLargeOperator() const;

Maybe we can take advantage to remove the "get" prefix and rename this to displayStyleLargeOperator().

[Frédéric Wang (:fredw)]

Comment on attachment 277058 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=277058&action=review

>> Source/WebCore/rendering/mathml/RenderMathMLOperator.h:146
>> +    StretchyData getDisplayStyleLargeOperator() const;
> 
> Maybe we can take advantage to remove the "get" prefix and rename this to displayStyleLargeOperator().

When this is moved to the MathOperator class it does not return a value but just finds such a large operator and set the MathOperator members immediately. And it actually get renamed findDisplayStyleLargeOperator. I plan to do this step in a next patch, so I don't think remove the get* prefix for now.

[Frédéric Wang (:fredw)]

Comment on attachment 277058 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=277058&action=review

>>> Source/WebCore/rendering/mathml/RenderMathMLOperator.h:146
>>> +    StretchyData getDisplayStyleLargeOperator() const;
>> 
>> Maybe we can take advantage to remove the "get" prefix and rename this to displayStyleLargeOperator().
> 
> When this is moved to the MathOperator class it does not return a value but just finds such a large operator and set the MathOperator members immediately. And it actually get renamed findDisplayStyleLargeOperator. I plan to do this step in a next patch, so I don't think remove the get* prefix for now.

The rename to findDisplayStyleLargeOperator and removal of return value is done on the patch for bug 156913.

[Frédéric Wang (:fredw)]

Created attachment 277469 [details]
Patch

Minor change: we avoid passing m_textContent to findStretchyData.

[Alejandro G. Castro]

Comment on attachment 277469 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=277469&action=review

LGTM

> Source/WebCore/ChangeLog:12
> +        (WebCore::RenderMathMLOperator::italicCorrection): We do not need to pass m_textContent to getDisplayStyleLargeOperator.
> +        (WebCore::RenderMathMLOperator::computePreferredLogicalWidths): We use getBaseGlyph and do not pass m_textContent to getDisplayStyleLargeOperator or findStretchyData.

Are these lines wrapped?

[Frédéric Wang (:fredw)]

Committed r200185: <http://trac.webkit.org/changeset/200185>

[WebKit Commit Bot]

Re-opened since this is blocked by bug 157131

[Frédéric Wang (:fredw)]

Committed r200187: <http://trac.webkit.org/changeset/200187>