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

[css-display] Use "computes to" instead of "behaves as" for display: contents on unusual elements. #2755

Closed
emilio opened this issue Jun 11, 2018 · 6 comments

Comments

@emilio
Copy link
Collaborator

@emilio emilio commented Jun 11, 2018

https://drafts.csswg.org/css-display/#unbox says:

display: contents behaves as display: none.

Which means that the computed value of display should still be contents. This adds unnecessary complexity, and this is not how all engines implementing this section right work.

In particular, it requires a mapping from element to "is really display contents", or something of the sort. Firefox used to have that, but it caused a lot of performance issues for no gain, so I removed it in https://bugzilla.mozilla.org/show_bug.cgi?id=1303605.

(Note that I removed that mapping earlier without realizing the correctness issue it introduced, https://bugzilla.mozilla.org/show_bug.cgi?id=1453702, which I fixed computing to none instead of contents, which also aligns with Blink and WebKit).

@emilio
Copy link
Collaborator Author

@emilio emilio commented Jun 11, 2018

cc @lilles since he implemented the Blink style fixup, but I think he wouldn't have any objection about changing this, given it would adapt the spec to how Blink, Firefox and WebKit implement.

@lilles
Copy link
Member

@lilles lilles commented Jun 11, 2018

Huh. I thought we did store the original display for computed style before adjusting. I think we're fine with either resolution.

@lilles
Copy link
Member

@lilles lilles commented Jun 11, 2018

I misunderstood our OriginalDisplay() thing. Not used for getComputedStyle(). I support resolving this as proposed.

@tabatkins
Copy link
Member

@tabatkins tabatkins commented Jun 11, 2018

Sounds fine to me. @fantasai, any objection?

@fantasai
Copy link
Collaborator

@fantasai fantasai commented Jun 11, 2018

WFM.

@fantasai fantasai added the Agenda+ label Jun 11, 2018
@css-meeting-bot
Copy link
Member

@css-meeting-bot css-meeting-bot commented Jun 27, 2018

The Working Group just discussed Use "computes to" instead of "behaves as" for display: contents on unusual elements, and agreed to the following:

  • RESOLVED: serialize it out as none
  • RESOLVED: compute to none
The full IRC log of that discussion <dael> Topic: Use "computes to" instead of "behaves as" for display: contents on unusual elements
<dael> github: https://github.com//issues/2755
<dael> fantasai: We had said that display contents behaves as display:none on certain elements like some SVG. There's an appendix of exactly which ones. emilio filed an issue to jsut say computes to display:none since that's easier from impl then handling that at used style time
<dael> fantasai: TabAtkins and I have not much of an opinion. Up to impl.
<dael> emilio: Do we clear everyone that impl computes to none?
<fantasai> s/Do we/To be/
<astearns> s/Do we/To be/
<dael> Rossen_: Any reason to do anything other than that? Sounds like there's interop.
<dael> Rossen_: Objections to serialize it out as none?
<dael> RESOLVED: serialize it out as none
<dael> RESOLVED: compute to none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants