Bug 159623

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:

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Bug Depends on:

108778, 133845, 159625

Bug Blocks:

156536, 160034

Attachments:

Description	Flags
Patch (applies after bug 159625)	none
Patch bug 159625 + 159623 (for EWS testing)	none
Patch	bfulgham: review+

Frédéric Wang (:fredw)

Reported 2016-07-11 07:14:09 PDT

This is bug 156536 for the attributes parsed in rendering/mathml/MathMLStyle.cpp

Attachments
Patch (applies after bug 159625) (33.98 KB, patch) 2016-07-20 07:09 PDT, Frédéric Wang (:fredw)	no flags	Details Formatted Diff Diff
Patch bug 159625 + 159623 (for EWS testing) (60.47 KB, patch) 2016-07-20 07:10 PDT, Frédéric Wang (:fredw)	no flags	Details Formatted Diff Diff
Patch (33.64 KB, patch) 2016-07-21 22:55 PDT, Frédéric Wang (:fredw)	bfulgham: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Frédéric Wang (:fredw)

Comment 1 2016-07-20 07:09:16 PDT

Created attachment 284101 [details] Patch (applies after bug 159625)

Frédéric Wang (:fredw)

Comment 2 2016-07-20 07:10:31 PDT

Created attachment 284102 [details] Patch bug 159625 + 159623 (for EWS testing)

Frédéric Wang (:fredw)

Comment 3 2016-07-21 22:55:43 PDT

Created attachment 284307 [details] Patch

Brent Fulgham

Comment 4 2016-07-22 09:28:03 PDT

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"

Brent Fulgham

Comment 5 2016-07-22 09:29:24 PDT

Comment on attachment 284307 [details] Patch Looks great! Thanks.

Darin Adler

Comment 6 2016-07-22 11:20:01 PDT

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.

Frédéric Wang (:fredw)

Comment 7 2016-07-25 00:19:58 PDT

Committed r203679: <http://trac.webkit.org/changeset/203679>

Note You need to log in before you can comment on or make changes to this bug.