Bug 216702

Summary: Implement the CSS math-style property
Product: WebKit Reporter: Frédéric Wang (:fredw) <fred.wang>
Component: CSSAssignee: Frédéric Wang (:fredw) <fred.wang>
Status: RESOLVED FIXED    
Severity: Normal CC: changseok, darin, dbarton, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, joepeck, kondapallykalyan, macpherson, menard, pdr, rbuis, rwlbuis, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugzilla.mozilla.org/show_bug.cgi?id=1665975
Bug Depends on: 216871    
Bug Blocks: 195797, 202303    
Attachments:
Description Flags
WIP Patch
none
WIP Patch
none
Patch
none
Patch
rbuis: review+
Patch
none
Patch none

Description Frédéric Wang (:fredw) 2020-09-18 10:04:26 PDT
See
https://mathml-refresh.github.io/mathml-core/#the-math-style-property
https://github.com/w3c/csswg-drafts/issues/5387

Currently this won't have effect outside MathML, since we don't implement math-level / scriptlevel.

Currently MathML's displaystyle is implemented using an internal property of Source/WebCore/rendering/mathml/MathMLStyle.h ; instead we could just replace it with math-style.
Comment 1 Frédéric Wang (:fredw) 2020-09-21 21:38:21 PDT
Created attachment 409348 [details]
WIP Patch
Comment 2 Frédéric Wang (:fredw) 2020-09-25 00:50:45 PDT
Created attachment 409665 [details]
WIP Patch
Comment 3 Frédéric Wang (:fredw) 2020-09-25 02:45:08 PDT
Created attachment 409675 [details]
Patch
Comment 4 Frédéric Wang (:fredw) 2020-09-25 04:55:16 PDT
Created attachment 409680 [details]
Patch
Comment 5 Frédéric Wang (:fredw) 2020-09-25 04:57:03 PDT
@rbuis: Please review the MathML part.

@Simon: Please review the CSS part, in particular do you it is worth guarding the new math-style property under a preference? Would that still allow to make the property usable internally when the flag is disabled?
Comment 6 Rob Buis 2020-09-25 06:07:14 PDT
Comment on attachment 409680 [details]
Patch

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

> Source/WebCore/ChangeLog:18
> +        [2]https://github.com/w3c/csswg-drafts/issues/5387

Add space.

> Source/WebCore/ChangeLog:36
> +        (mtable):

Not sure if these are needed, it is allowed to edit exact changes out of ChangeLogs.
Comment 7 Frédéric Wang (:fredw) 2020-09-25 06:27:30 PDT
Created attachment 409685 [details]
Patch
Comment 8 Simon Fraser (smfr) 2020-09-25 09:44:37 PDT
Comment on attachment 409685 [details]
Patch

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

> Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp:149
>  bool RenderMathMLUnderOver::shouldMoveLimits()

Can this function be const?
Comment 9 Frédéric Wang (:fredw) 2020-09-25 10:00:21 PDT
Comment on attachment 409685 [details]
Patch

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

>> Source/WebCore/rendering/mathml/RenderMathMLUnderOver.cpp:149
>>  bool RenderMathMLUnderOver::shouldMoveLimits()
> 
> Can this function be const?

Right, it can. will do that.
Comment 10 Radar WebKit Bug Importer 2020-09-25 10:05:38 PDT
<rdar://problem/69578233>
Comment 11 Frédéric Wang (:fredw) 2020-09-25 10:14:18 PDT
Created attachment 409704 [details]
Patch
Comment 12 EWS 2020-09-25 10:47:37 PDT
Committed r267578: <https://trac.webkit.org/changeset/267578>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 409704 [details].
Comment 13 Darin Adler 2020-09-26 16:12:57 PDT
Comment on attachment 409680 [details]
Patch

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

>> Source/WebCore/ChangeLog:36
>> +        (mtable):
> 
> Not sure if these are needed, it is allowed to edit exact changes out of ChangeLogs.

Here’s another way to say this:

The scripts that prepare change logs are there to help contributors write a good change logs. But the person submitting the patch is still the author, not the script. Constributors should feel free to edit to make the log good, correct, and intentional.

This is difficult, though, if a contributor does not "buy in" to the way the WebKit project does change logs. If someone doesn’t then they probably will think of this as boilerplate, or unnecessary garbage, and likely leave it as is without even looking it over. The idea when the project began, inherited from past open source projects, was to write a per-function comment to help clarify what the change is doing and why. But it seems that few people think that’s worthwhile at this point. So maybe we will change our approach.