WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
159623
Move parsing of display, displaystyle and mathvariant attributes from rendering to mathml element classes
https://bugs.webkit.org/show_bug.cgi?id=159623
Summary
Move parsing of display, displaystyle and mathvariant attributes from renderi...
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.
Top of Page
Format For Printing
XML
Clone This Bug