Bug 216702 - Implement the CSS math-style property
Summary: Implement the CSS math-style property
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Frédéric Wang (:fredw)
URL:
Keywords: InRadar
Depends on: 216871
Blocks: mathml-core 202303
  Show dependency treegraph
 
Reported: 2020-09-18 10:04 PDT by Frédéric Wang (:fredw)
Modified: 2020-09-28 04:42 PDT (History)
16 users (show)

See Also:


Attachments
WIP Patch (31.01 KB, patch)
2020-09-21 21:38 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff
WIP Patch (31.25 KB, patch)
2020-09-25 00:50 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff
Patch (63.64 KB, patch)
2020-09-25 02:45 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff
Patch (226.36 KB, patch)
2020-09-25 04:55 PDT, Frédéric Wang (:fredw)
rbuis: review+
Details | Formatted Diff | Diff
Patch (228.69 KB, patch)
2020-09-25 06:27 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff
Patch (229.36 KB, patch)
2020-09-25 10:14 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.