Bug 157071

Summary: RenderMathOperator: Move calculation of preferred width into MathOperator
Product: WebKit Reporter: Frédéric Wang (:fredw) <fred.wang>
Component: MathMLAssignee: Frédéric Wang (:fredw) <fred.wang>
Status: RESOLVED FIXED    
Severity: Normal CC: alex, bfulgham, cfleizach, commit-queue, darin, dbarton, esprehn+autocc, glenn, kondapallykalyan, mrobinson, rego, svillar
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 156910, 156913, 156921, 156950    
Bug Blocks: 152244    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch for EWS testing
none
Patch
none
Patch bfulgham: review+

Description Frédéric Wang (:fredw) 2016-04-27 07:18:18 PDT
I'm extracting more changes from bug 152244.
Comment 1 Frédéric Wang (:fredw) 2016-04-27 07:32:50 PDT
Created attachment 277476 [details]
Patch
Comment 2 Frédéric Wang (:fredw) 2016-05-09 00:51:53 PDT
Created attachment 278392 [details]
Patch

Updating after https://trac.webkit.org/changeset/200569
Comment 3 Frédéric Wang (:fredw) 2016-05-11 01:53:34 PDT
Created attachment 278602 [details]
Patch

Minor change: s/maximumGlyphWidth/setMaxPreferredWidth/ in the ASSERT of MathOperator::calculateStretchyData.
Comment 4 Frédéric Wang (:fredw) 2016-05-11 02:19:12 PDT
Created attachment 278606 [details]
Patch for EWS testing
Comment 5 Manuel Rego Casasnovas 2016-05-13 08:21:17 PDT
Comment on attachment 278602 [details]
Patch

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

LGTM just some minor nits.

> Source/WebCore/ChangeLog:11
> +        (WebCore::MathOperator::MathOperator): Initialize m_maxPreferredWidth

Nit: Missing dot at the end.

> Source/WebCore/ChangeLog:21
> +        (WebCore::RenderMathMLOperator::computePreferredLogicalWidths): This function performs bad

Nit: s/bad/wrong/

> Source/WebCore/rendering/mathml/MathOperator.cpp:264
> +void MathOperator::calculateStretchyData(const RenderStyle& style, bool setMaxPreferredWidth, LayoutUnit targetSize)

A small suggestion, maybe call the argument "calculateMaxPreferredWidth".

> Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp:270
> +    // FIXME: We should not using stretchSize during the preferred width calculation nor should we leave logical width dirty (http://webkit.org/b/152244).

Nit: s/using/use/
Comment 6 Frédéric Wang (:fredw) 2016-05-13 09:42:55 PDT
Created attachment 278842 [details]
Patch

Addressing Manuel's comments...
Comment 7 Frédéric Wang (:fredw) 2016-06-09 07:57:32 PDT
Created attachment 280918 [details]
Patch
Comment 8 Brent Fulgham 2016-06-09 13:32:50 PDT
Comment on attachment 280918 [details]
Patch

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

Looks like a nice clean-up. r=me.

> Source/WebCore/rendering/mathml/MathOperator.h:93
> +    LayoutUnit m_maxPreferredWidth = 0;

We prefer the C++11 initializer syntax, but I guess you are being consistent with the other things in this file.
Comment 9 Frédéric Wang (:fredw) 2016-06-09 13:49:24 PDT
Committed r201881: <http://trac.webkit.org/changeset/201881>