[Webkit-unassigned] [Bug 155406] MathMLMencloseElement style should not depend on rendering

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=155406

--- Comment #5 from Darin Adler <darin at apple.com> ---
Comment on attachment 273889
  --> https://bugs.webkit.org/attachment.cgi?id=273889
patch

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

> Source/WebCore/mathml/MathMLMencloseElement.cpp:134
>      String closingBrace(")", String::ConstructFromLiteral);
>      TextRun run(closingBrace);

We do not need a String to construct a TextRun, which takes a StringView. We can write something more like this:

    LChar rightParenthesis = ')';
    TextRun run({ &rightParenthesis, 1 });

I took the liberty of calling this what the Unicode standard calls it, rather than calling it "closing brace", which seems like it could be the name of some other characters.

> Source/WebCore/mathml/MathMLMencloseElement.cpp:136
> +    const FontCascade& font = parentStyle.fontCascade();
> +    padding.appendNumber(font.width(run));

I don’t think we need a local variable when we are just using it once. In fact, I would write this:

    LChar rightParenthesis = ')';
    padding.appendNumber(parentStyle.fontCascade().width(TextRun{ &rightParenthesis, 1 }));

Might need an extra type name or set of curly brackets in there, but something like that would work. The version with more lines of code seems no more clear to me.

> Source/WebCore/mathml/MathMLMencloseElement.cpp:143
> +String MathMLMencloseElement::longDivLeftPadding() const

This could return a const String& instead of a String to avoid unneeded reference count churn.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160313/d7a48967/attachment.html>