Summary: | Move parsing of display, displaystyle and mathvariant attributes from rendering to mathml element classes | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Frédéric Wang (:fredw) <fred.wang> | ||||||||
Component: | MathML | Assignee: | Frédéric Wang (:fredw) <fred.wang> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | bfulgham, commit-queue, darin, dbarton, esprehn+autocc, glenn, kondapallykalyan | ||||||||
Priority: | P2 | ||||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Bug Depends on: | 108778, 133845, 159625 | ||||||||||
Bug Blocks: | 156536, 160034 | ||||||||||
Attachments: |
|
Description
Frédéric Wang (:fredw)
2016-07-11 07:14:09 PDT
Created attachment 284101 [details] Patch (applies after bug 159625) Created attachment 284102 [details] Patch bug 159625 + 159623 (for EWS testing) Created attachment 284307 [details]
Patch
Comment on attachment 284307 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=284307&action=review > Source/WebCore/mathml/MathMLElement.cpp:589 > +void MathMLElement::getSpecifiedDisplayStyle(bool& displayStyle) We avoid using 'get' names in WebKit. This should be called "specifiedDisplayStyle". > Source/WebCore/mathml/MathMLElement.cpp:598 > +void MathMLElement::getSpecifiedMathVariant(MathMLElement::MathVariant& mathVariant) Ditto. We should probably call this "specifiedMathVariant" Comment on attachment 284307 [details]
Patch
Looks great! Thanks.
Comment on attachment 284307 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=284307&action=review >> Source/WebCore/mathml/MathMLElement.cpp:589 >> +void MathMLElement::getSpecifiedDisplayStyle(bool& displayStyle) > > We avoid using 'get' names in WebKit. This should be called "specifiedDisplayStyle". We actually reserve the use of get for functions with out arguments. And there is an out argument here. But it’s peculiar how this works; it sets the argument to true or false, or possibly leaves the argument alone in other cases. I don’t think the name "get" makes that kind of unconventional behavior clear enough. I think it would be clearer to use a return type of Optional<bool> for clarity even if that usage is less economical at the call site. Then the function would indeed be named specifiedDisplayStyle. >> Source/WebCore/mathml/MathMLElement.cpp:598 >> +void MathMLElement::getSpecifiedMathVariant(MathMLElement::MathVariant& mathVariant) > > Ditto. We should probably call this "specifiedMathVariant" Same comment as above. Committed r203679: <http://trac.webkit.org/changeset/203679> |