Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define how margin-collapsing should behave #42

Closed
bfgeek opened this issue Feb 25, 2019 · 9 comments
Closed

Define how margin-collapsing should behave #42

bfgeek opened this issue Feb 25, 2019 · 9 comments

Comments

@bfgeek
Copy link

bfgeek commented Feb 25, 2019

Today the current implementations work differently when it comes to margin collapsing.

A simple case is:
https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=6687

In the above example Firefox appears the formatting context behaves like a block formatting context in that it abides by block-like margin collapsing rules.

I'm not sure what Safari is doing. E.g. another example:
https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=6688

@bfgeek
Copy link
Author

bfgeek commented Feb 25, 2019

My preference here would be to drop any margin collapsing rules in implementations, and instead force completely new formatting contexts on each element.

This needs a lot of tests (with various combinations of elements, positive/negative margins, various levels of nesting) to ensure compatibility.

@fred-wang fred-wang added the css / html5 Issues related to CSS or HTML5 interoperability label Feb 25, 2019
@fred-wang fred-wang added the need resolution Issues needing resolution at MathML Refresh CG meeting label May 16, 2019
@fred-wang
Copy link

The core spec does not mention any margin collapsing rules currently (although maybe they are implicit in mtable and mtext layout). I'm not sure that would be useful for math.

@fred-wang
Copy link

Another test case to compare against CSS, so people can understand:
https://people.igalia.com/fwang/mathml-layout-ng/mathml-margin-collapsing.html

We have:

  • a parent (green) with 30px margin-top and 60px margin-bottom
  • a child (blue) with 30px margin-top and 60px margin-bottom

Should MathML behave like:

  • block: the parent & child's top margins (respectively bottom margins) are collapsed into a single 30px space (respectively 60px space).

  • or inline block: we have margin-top (respectively margin-bottom) from the child (green) and parent (white) summing up to a 60px space (respectively 120px space).
    ?

  • Currently Gecko behaves like block and performs margin collapsing.

  • Baseline alignment seems broken in WebKit when the children have non-zero margin. However, from the added space it seems it does applies margin collapsing rules.

  • Igalia's chromium build currently ignores margin values (cf Ensure good integration of MathML with the accessibility tree #9), so it's difficult to say anything about margin collapsing for now. But my guess is that by default the block behavior will apply, because we rely on block flow layout classes for most MathML elements.

As I said, margin collapsing probably does not make any sense for math layout. However, if we don't really care about it and if that facilitates the implementations to follow block's behavior maybe we actually want that.

Adding this to the mathml core meeting agenda.

@NSoiffer
Copy link
Contributor

The <math> element has a display property which can be either block or inline. Shouldn't the behavior depend upon whether MathML this property is set to a specific value (assuming MathML will support margins)? Or maybe it should be harmonized/be declared the same as CSS's display property that allows many more options such as none and inline-block (most don't make much sense for MathML such as 'table', 'list-item', etc).

I don't see the case for inline-block. Is it when the MathML is inside of an inline-block context and display is not set?

Note: if display is not set, the spec is currently silent on what should happen, so that should probably be fixed. Based on the user agent stylesheet, it looks like inline was chosen as the default.

@fred-wang
Copy link

Regarding new displays, there is a separate discussion #56 ; currently the actual display value is browser-specific. If the user specifies other values, the idea in #56 would be to fallback to CSS rather than MathML layout.

For the <math> which has "display: block" or "display: inline" we should probably follow CSS for margin collapsing. I want to introduce an anonymous mrow child, so the math root can still layout its child as an mrow.

Anyway, for this issue the only thing to discuss is whether margin-collapsing make any sense for math-specific layout or not.

@bfgeek
Copy link
Author

bfgeek commented Jul 16, 2019

I'd err on the side of leaving margin collapsing out. As it has a lot of weird side affects for the new algorithms/display-types, and is very difficult to get right. E.g. do margins for these new types collapse through if they are empty?
https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=7062

If you define for these new types that they establish new formatting contexts or their particular type, and their children also establish new formatting context or their particular type, you only need to define how margins interact within that display type.

@fred-wang
Copy link

Consensus during mathml core meeting: Ignore margin-collapsing for all new layout as they are no clear use case and that simplifies implementation.

fred-wang added a commit to web-platform-tests/wpt that referenced this issue Jul 30, 2019
fred-wang added a commit to web-platform-tests/wpt that referenced this issue Jul 30, 2019
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Aug 5, 2019
…, a=testonly

Automatic update from web-platform-tests
MathML: Add tests for margin-collapsing. (#18170)

See w3c/mathml#42
--

wpt-commits: bf7e393610c462cd671feb88952330db442f8499
wpt-pr: 18170
xeonchen pushed a commit to xeonchen/gecko that referenced this issue Aug 5, 2019
…, a=testonly

Automatic update from web-platform-tests
MathML: Add tests for margin-collapsing. (#18170)

See w3c/mathml#42
--

wpt-commits: bf7e393610c462cd671feb88952330db442f8499
wpt-pr: 18170
@fred-wang fred-wang added need tests Issues related to writing WPT tests MathML Core Issues affecting the MathML Core specification need implementation update and removed need tests Issues related to writing WPT tests need resolution Issues needing resolution at MathML Refresh CG meeting labels Sep 16, 2019
@fred-wang
Copy link

There is one test for margin-collapsing: https://w3c-test.org/mathml/relations/css-styling/padding-border-margin/margin-002.html ; maybe we can add more.

@fred-wang fred-wang added need tests Issues related to writing WPT tests and removed need implementation update labels Sep 16, 2019
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 4, 2019
…, a=testonly

Automatic update from web-platform-tests
MathML: Add tests for margin-collapsing. (#18170)

See w3c/mathml#42
--

wpt-commits: bf7e393610c462cd671feb88952330db442f8499
wpt-pr: 18170

UltraBlame original commit: 1686e99069d46703832f9e3d57effcc1ce2871f4
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 4, 2019
…, a=testonly

Automatic update from web-platform-tests
MathML: Add tests for margin-collapsing. (#18170)

See w3c/mathml#42
--

wpt-commits: bf7e393610c462cd671feb88952330db442f8499
wpt-pr: 18170

UltraBlame original commit: 1686e99069d46703832f9e3d57effcc1ce2871f4
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 4, 2019
…, a=testonly

Automatic update from web-platform-tests
MathML: Add tests for margin-collapsing. (#18170)

See w3c/mathml#42
--

wpt-commits: bf7e393610c462cd671feb88952330db442f8499
wpt-pr: 18170

UltraBlame original commit: 1686e99069d46703832f9e3d57effcc1ce2871f4
@NSoiffer NSoiffer removed MathML Core Issues affecting the MathML Core specification css / html5 Issues related to CSS or HTML5 interoperability need tests Issues related to writing WPT tests labels Mar 2, 2020
@NSoiffer
Copy link
Contributor

NSoiffer commented Mar 2, 2020

Since you only are testing that nothing should happen, so one test is fine.

@NSoiffer NSoiffer closed this as completed Mar 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants