Bug 157071 - RenderMathOperator: Move calculation of preferred width into MathOperator
Summary: RenderMathOperator: Move calculation of preferred width into MathOperator
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: MathML (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Frédéric Wang (:fredw)
URL:
Keywords:
Depends on: 156910 156913 156921 156950
Blocks: 152244
  Show dependency tree / graph
 
Reported: 2016-04-27 07:18 PDT by Frédéric Wang (:fredw)
Modified: 2016-06-09 13:49 PDT (History)
12 users (show)

See Also:

Attachments
Patch (11.91 KB, patch)
2016-04-27 07:32 PDT, Frédéric Wang (:fredw) 		no flags Details | Formatted Diff | Diff
Patch (12.77 KB, patch)
2016-05-09 00:51 PDT, Frédéric Wang (:fredw) 		no flags Details | Formatted Diff | Diff
Patch (12.86 KB, patch)
2016-05-11 01:53 PDT, Frédéric Wang (:fredw) 		no flags Details | Formatted Diff | Diff
Patch for EWS testing (80.40 KB, patch)
2016-05-11 02:19 PDT, Frédéric Wang (:fredw) 		no flags Details | Formatted Diff | Diff
Patch (12.56 KB, patch)
2016-05-13 09:42 PDT, Frédéric Wang (:fredw) 		no flags Details | Formatted Diff | Diff
Patch (12.52 KB, patch)
2016-06-09 07:57 PDT, Frédéric Wang (:fredw) 		bfulgham: review+
Details | Formatted Diff | Diff
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>