Bug 152242 - Move MathMLOperatorDictionary features into a separate module
Summary: Move MathMLOperatorDictionary features into a separate module
Alias: None
Product: WebKit
Classification: Unclassified
Component: MathML (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Frédéric Wang (:fredw)
Depends on:
Blocks: 152244 159619
  Show dependency treegraph
Reported: 2015-12-14 01:28 PST by Frédéric Wang (:fredw)
Modified: 2016-07-11 05:25 PDT (History)
2 users (show)

See Also:

Patch (161.43 KB, patch)
2015-12-14 01:55 PST, Frédéric Wang (:fredw)
mrobinson: review+
mrobinson: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Frédéric Wang (:fredw) 2015-12-14 01:28:17 PST
The RenderMathMLOperator class contains three independent features:
1) The management of the renderer for operator
2) The search in the MathML operator dictionary
3) The selection/measuring/drawing of size variants & glyph assembly for stretchy operators

It is cleaner and more easily readable/manageable for future improvements to do as in Gecko and split these features into separate modules.

Also having 3) in a separate module will allow to draw radical or longdiv symbols without creating an anonymous renderer.

This bug is about the first step move 2) into its own module.
Comment 1 Frédéric Wang (:fredw) 2015-12-14 01:55:23 PST
Created attachment 267287 [details]
Comment 2 Martin Robinson 2015-12-14 02:03:35 PST
Comment on attachment 267287 [details]

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

Looks fine to me, though I do have a few nits about the coding style.

> Source/WebCore/rendering/mathml/MathMLOperatorDictionary.cpp:1105
> +        if (entry != dictionary && (entry-1)->character == textContent)
> +            entry--;
> +        if (entry != dictionary && (entry-1)->character == textContent)
> +            entry--;

The binary operators should have spaces around them, thus: "entry - 1"

> Source/WebCore/rendering/mathml/MathMLOperatorDictionary.cpp:1113
> +    return !(tryBinarySearch<const UChar, UChar>(horizontalOperators, WTF_ARRAY_LENGTH(horizontalOperators), textContent, ExtractKeyHorizontal));

Is the outermost pair of parenthesis necessary here? If not, might be better to remove it.
Comment 3 Frédéric Wang (:fredw) 2015-12-14 03:49:19 PST
Committed r194031: <http://trac.webkit.org/changeset/194031>