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

[css-display] Define how display: contents behaves in MathML. #2167

Closed
emilio opened this Issue Jan 6, 2018 · 7 comments

Comments

Projects
None yet
8 participants
@emilio
Collaborator

emilio commented Jan 6, 2018

https://drafts.csswg.org/css-display/#unbox defines how display: contents behaves for unusual HTML elements and for SVG elements, but not MathML.

It'd be nice to define how they should behave, even if it's just "it behaves normally".

@tabatkins

This comment has been minimized.

Member

tabatkins commented Mar 21, 2018

From my reading of MathML3, there's approximately zero elements where it makes sense to use display:contents, so for now I'm defining that it is treated as "display: none" for all of them.

@tabatkins tabatkins closed this in 1d3e6a0 Mar 21, 2018

@fantasai fantasai added the Agenda+ label Mar 21, 2018

@fantasai fantasai reopened this Mar 21, 2018

@frivoal

This comment has been minimized.

Collaborator

frivoal commented Mar 22, 2018

Randomly pulling in someone who ought to know a thing or two about MathML: @pkra, what do you think about the above?

@fred-wang

This comment has been minimized.

fred-wang commented Mar 23, 2018

I discussed this a bit at Igalia. Here is a concrete example:

<math>
  <mfrac>
    <mrow style="background: green; color: red; display: contents">
      <mi>x</mi>
      <mo>+</mo>
      <mi>y</mi>
    </mrow>
    <mn>2</mn>
  </mfrac>
</math>

WebKit and Gecko currently ignore "display: contents" so you will see the fraction numerator with green background.

If instead you treat "display: contents" as "display: none" then it will look like this:

<math>
  <mfrac>
    <mrow style="display: none">
      <mi>x</mi>
      <mo>+</mo>
      <mi>y</mi>
    </mrow>
    <mn>2</mn>
  </mfrac>
</math>

Gecko and WebKit don't create a box for the numerator and so the fraction is rendered as invalid markup (WebKit actually displays nothing because of https://bugs.webkit.org/show_bug.cgi?id=123348) which is different from the current implementation of display: contents. I guess it's easy to implement, just say "display: none" won't create layout/renderer classes and the MathML code will take care of the rest.

Another option is to basically "ignore the math layout" so

<math>
  <mfrac style="background: green; color: red; display: contents">
    <mi>x</mi>
    <mi>y</mi>
  </mfrac>
</math>

is treated as just

<math>
  <mi style="color: red;">x</mi>
  <mi style="color: red;">y</mi>
</math>

with the fraction and background ignored. But things might become more complicated to understand and implement (the first example would be treated as a fraction with 4 elements so invalid) and it's not clear whether there is a concrete use case in practice.

Note that we already have similiar situations like borders or absolute positioning where Gecko or WebKit just ignore CSS properties or choose the option that is the most convenient for implementers. So it seems either keeping "display: content" as it (i.e. ignoring it in MathML element) or changing implementations to match "display: none" is the most appropriate option for MathML.

@emilio: What is your opinion on this? What will be the easiest for Mozilla?

@MatsPalmgren

This comment has been minimized.

MatsPalmgren commented Mar 23, 2018

Handling it as display: none seems like the best option to me. It should be easy to implement for us since display: none is already supported.

@tabatkins

This comment has been minimized.

Member

tabatkins commented Mar 23, 2018

Gecko and WebKit don't create a box for the numerator and so the fraction is rendered as invalid markup

With it acting as display:none, the fraction is invalid because there's only one argument. With it acting as display:contents, the fraction would also be invalid, because there's 4 arguments.

This sort of thing abounds in MathML; every place I saw where I thought display:contents might make sense, it would just as often make things invalid, too. This sort of consideration doesn't apply to HTML's or SVG's rules.

The closest thing we could get to the intent of display:contents would be something like "treat it like an unstyled mrow instead", but that's both complicated (and I don't think it'll work everywhere) and more importantly, not what display:contents does.

So yeah, absent the WG deciding on something totally different, display:none for everything seems best. ^_^

@pkra

This comment has been minimized.

Member

pkra commented Mar 23, 2018

Randomly pulling in someone who ought to know a thing or two about MathML: @pkra, what do you think about the above?

I don't know anyone relying on native MathML implementations in a professional production setting (I haven't checked crawler data in a while but it usually confirms this quite broadly). Mixing MathML and CSS is extremely rare in general since MathML usually serves in XML-to-print workflows as well.

So whatever is easiest seems very reasonable.

@css-meeting-bot

This comment has been minimized.

Member

css-meeting-bot commented Apr 4, 2018

The Working Group just discussed ] 'display: contents' on MathML, and agreed to the following resolutions:

  • RESOLVED: treating display:contents as display:none for MathML
The full IRC log of that discussion <dael_> Topic: ] 'display: contents' on MathML
<dael_> github: https://github.com//issues/2167
<dael_> TabAtkins: We have special ruels for how display:content should work in SVG. [missed]
<dael_> TabAtkins: How to handle display:contents a few elements treat it like HTML, ones with a transparent content model. Everything else is treated as display:none because children can't be rendered one level up. Looking at mathML there are 0 instances where children can be rendered in outer context.
<dael_> TabAtkins: It would almost never be correct to lift children to outer context. There are a few places where it's right with only a certain number of children. Best thing I can see is make it always display:none on MathML. Firefox des were fine.
<florian> +1
<dael_> astearns: Given other times we said display:contents are handled as display:none and how well they went over I expect this to come up again.
<dael_> TabAtkins: If someone can point to a mathML fucntion where it makes sense we can change this.
<dael_> fantasai: We'll CC mathML WG when we publish a WD
<dael_> florian: And we CCed some MathML people along the way.
<dael_> astearns: You want to talk to math on the web group
<dael_> astearns: Obj ot treating display:contents as display:none for MathML?
<dael_> RESOLVED: treating display:contents as display:none for MathML
<dael_> Topic

@css-meeting-bot css-meeting-bot removed the Agenda+ label Apr 4, 2018

@fantasai fantasai closed this Apr 5, 2018

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Apr 16, 2018

Bug 1453702: [css-display] Move unboxing to style, and handle display…
…: contents before other suppressions. r=mats,xidorn

This also adopts the resolution of [1] while at it, and switches XUL to not
support display: contents until a use case appears.

This makes our behavior consistent both with the spec and also in terms of
handling dynamic changes to stuff that would otherwise get suppressed.

Also makes us consistent with both Blink and WebKit in terms of computed style.
We were the only ones respecting "behaves as display: none" without actually
computing to display: none. Will file a spec issue to get that changed.

It also makes us match Blink and WebKit in terms of respecting display: contents
before other suppressions, see the reftest which I didn't write as a WPT
(because there's no spec supporting neither that or the opposite of what we do),
where a <g> element respects display: contents even though if it had any other
kind of display value we'd suppress the frame for it and all the descendants
since it's an SVG element in a non-SVG subtree.

Also, this removes the page-break bit from the display: contents loop, which I
think is harmless.

As long as the tests under style are based in namespace id / node name /
traversal parent, this should not make style sharing go wrong in any way, since
that's the first style sharing check we do at [2].

The general idea under this change is making all nodes with computed style of
display: contents actually honor it. Otherwise there's no way of making the
setup sound except re-introducing something similar to all the state tracking
removed in bug 1303605.

[1]: w3c/csswg-drafts#2167
[2]: https://searchfox.org/mozilla-central/rev/fca4426325624fecbd493c31389721513fc49fef/servo/components/style/sharing/mod.rs#700

MozReview-Commit-ID: JoCKnGYEleD

moz-wptsync-bot added a commit to web-platform-tests/wpt that referenced this issue Apr 16, 2018

css-display] Move unboxing to style, and handle display: contents bef…
…ore other suppressions.

This also adopts the resolution of [1] while at it, and switches XUL to not
support display: contents until a use case appears.

This makes our behavior consistent both with the spec and also in terms of
handling dynamic changes to stuff that would otherwise get suppressed.

Also makes us consistent with both Blink and WebKit in terms of computed style.
We were the only ones respecting "behaves as display: none" without actually
computing to display: none. Will file a spec issue to get that changed.

It also makes us match Blink and WebKit in terms of respecting display: contents
before other suppressions, see the reftest which I didn't write as a WPT
(because there's no spec supporting neither that or the opposite of what we do),
where a <g> element respects display: contents even though if it had any other
kind of display value we'd suppress the frame for it and all the descendants
since it's an SVG element in a non-SVG subtree.

Also, this removes the page-break bit from the display: contents loop, which I
think is harmless.

As long as the tests under style are based in namespace id / node name /
traversal parent, this should not make style sharing go wrong in any way, since
that's the first style sharing check we do at [2].

The general idea under this change is making all nodes with computed style of
display: contents actually honor it. Otherwise there's no way of making the
setup sound except re-introducing something similar to all the state tracking
removed in bug 1303605.

[1]: w3c/csswg-drafts#2167
[2]: https://searchfox.org/mozilla-central/rev/fca4426325624fecbd493c31389721513fc49fef/servo/components/style/sharing/mod.rs#700
bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1453702
gecko-commit: 43862b2ee72eec858f509ce3b9cb5c6fa96194ba
gecko-integration-branch: mozilla-inbound
gecko-reviewers: mats, xidorn

mykmelez pushed a commit to mozilla/gecko that referenced this issue Apr 17, 2018

Bug 1453702: [css-display] Move unboxing to style, and handle display…
…: contents before other suppressions. r=mats,xidorn

This also adopts the resolution of [1] while at it, and switches XUL to not
support display: contents until a use case appears.

This makes our behavior consistent both with the spec and also in terms of
handling dynamic changes to stuff that would otherwise get suppressed.

Also makes us consistent with both Blink and WebKit in terms of computed style.
We were the only ones respecting "behaves as display: none" without actually
computing to display: none. Will file a spec issue to get that changed.

It also makes us match Blink and WebKit in terms of respecting display: contents
before other suppressions, see the reftest which I didn't write as a WPT
(because there's no spec supporting neither that or the opposite of what we do),
where a <g> element respects display: contents even though if it had any other
kind of display value we'd suppress the frame for it and all the descendants
since it's an SVG element in a non-SVG subtree.

Also, this removes the page-break bit from the display: contents loop, which I
think is harmless.

As long as the tests under style are based in namespace id / node name /
traversal parent, this should not make style sharing go wrong in any way, since
that's the first style sharing check we do at [2].

The general idea under this change is making all nodes with computed style of
display: contents actually honor it. Otherwise there's no way of making the
setup sound except re-introducing something similar to all the state tracking
removed in bug 1303605.

[1]: w3c/csswg-drafts#2167
[2]: https://searchfox.org/mozilla-central/rev/fca4426325624fecbd493c31389721513fc49fef/servo/components/style/sharing/mod.rs#700

MozReview-Commit-ID: JoCKnGYEleD

moz-wptsync-bot added a commit to web-platform-tests/wpt that referenced this issue Apr 17, 2018

css-display] Move unboxing to style, and handle display: contents bef…
…ore other suppressions.

This also adopts the resolution of [1] while at it, and switches XUL to not
support display: contents until a use case appears.

This makes our behavior consistent both with the spec and also in terms of
handling dynamic changes to stuff that would otherwise get suppressed.

Also makes us consistent with both Blink and WebKit in terms of computed style.
We were the only ones respecting "behaves as display: none" without actually
computing to display: none. Will file a spec issue to get that changed.

It also makes us match Blink and WebKit in terms of respecting display: contents
before other suppressions, see the reftest which I didn't write as a WPT
(because there's no spec supporting neither that or the opposite of what we do),
where a <g> element respects display: contents even though if it had any other
kind of display value we'd suppress the frame for it and all the descendants
since it's an SVG element in a non-SVG subtree.

Also, this removes the page-break bit from the display: contents loop, which I
think is harmless.

As long as the tests under style are based in namespace id / node name /
traversal parent, this should not make style sharing go wrong in any way, since
that's the first style sharing check we do at [2].

The general idea under this change is making all nodes with computed style of
display: contents actually honor it. Otherwise there's no way of making the
setup sound except re-introducing something similar to all the state tracking
removed in bug 1303605.

[1]: w3c/csswg-drafts#2167
[2]: https://searchfox.org/mozilla-central/rev/fca4426325624fecbd493c31389721513fc49fef/servo/components/style/sharing/mod.rs#700
bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1453702
gecko-commit: 43862b2ee72eec858f509ce3b9cb5c6fa96194ba
gecko-integration-branch: mozilla-inbound
gecko-reviewers: mats, xidorn

emilio added a commit to emilio/servo that referenced this issue Apr 17, 2018

style: Move unboxing to style, and handle display: contents before ot…
…her suppressions.

This also adopts the resolution of [1] while at it, and switches XUL to not
support display: contents until a use case appears.

This makes our behavior consistent both with the spec and also in terms of
handling dynamic changes to stuff that would otherwise get suppressed.

Also makes us consistent with both Blink and WebKit in terms of computed style.
We were the only ones respecting "behaves as display: none" without actually
computing to display: none. Will file a spec issue to get that changed.

It also makes us match Blink and WebKit in terms of respecting display: contents
before other suppressions, see the reftest which I didn't write as a WPT
(because there's no spec supporting neither that or the opposite of what we do),
where a <g> element respects display: contents even though if it had any other
kind of display value we'd suppress the frame for it and all the descendants
since it's an SVG element in a non-SVG subtree.

Also, this removes the page-break bit from the display: contents loop, which I
think is harmless.

As long as the tests under style are based in namespace id / node name /
traversal parent, this should not make style sharing go wrong in any way, since
that's the first style sharing check we do at [2].

The general idea under this change is making all nodes with computed style of
display: contents actually honor it. Otherwise there's no way of making the
setup sound except re-introducing something similar to all the state tracking
removed in bug 1303605.

[1]: w3c/csswg-drafts#2167
[2]: https://searchfox.org/mozilla-central/rev/fca4426325624fecbd493c31389721513fc49fef/servo/components/style/sharing/mod.rs#700

Bug: 1453702
Reviewed-by: mats, xidorn
MozReview-Commit-ID: JoCKnGYEleD

Moggers added a commit to Moggers/servo that referenced this issue Jun 13, 2018

style: Move unboxing to style, and handle display: contents before ot…
…her suppressions.

This also adopts the resolution of [1] while at it, and switches XUL to not
support display: contents until a use case appears.

This makes our behavior consistent both with the spec and also in terms of
handling dynamic changes to stuff that would otherwise get suppressed.

Also makes us consistent with both Blink and WebKit in terms of computed style.
We were the only ones respecting "behaves as display: none" without actually
computing to display: none. Will file a spec issue to get that changed.

It also makes us match Blink and WebKit in terms of respecting display: contents
before other suppressions, see the reftest which I didn't write as a WPT
(because there's no spec supporting neither that or the opposite of what we do),
where a <g> element respects display: contents even though if it had any other
kind of display value we'd suppress the frame for it and all the descendants
since it's an SVG element in a non-SVG subtree.

Also, this removes the page-break bit from the display: contents loop, which I
think is harmless.

As long as the tests under style are based in namespace id / node name /
traversal parent, this should not make style sharing go wrong in any way, since
that's the first style sharing check we do at [2].

The general idea under this change is making all nodes with computed style of
display: contents actually honor it. Otherwise there's no way of making the
setup sound except re-introducing something similar to all the state tracking
removed in bug 1303605.

[1]: w3c/csswg-drafts#2167
[2]: https://searchfox.org/mozilla-central/rev/fca4426325624fecbd493c31389721513fc49fef/servo/components/style/sharing/mod.rs#700

Bug: 1453702
Reviewed-by: mats, xidorn
MozReview-Commit-ID: JoCKnGYEleD
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment