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] Define interaction of display:contents and replaced elements #540

Closed
zcorpan opened this issue Sep 28, 2016 · 40 comments
Closed

Comments

@zcorpan
Copy link
Member

zcorpan commented Sep 28, 2016

https://drafts.csswg.org/css-display/#box-generation

w3c/csswg-test#1129

It was not clear to me from the spec what the expected result of applying display:contents to replaced elements is. The added test for replace contents matches what Gecko implemented.

The implemented behavior seems to be that display:contents on replaced elements and form controls (not all of those are actually replaced elements) is ignored, as far as I can tell.

cc @rune-opera @tabatkins @fantasai

@zcorpan zcorpan added the css-display-3 Current Work label Sep 28, 2016
@MatsPalmgren
Copy link

MatsPalmgren commented Sep 28, 2016

We ignore display:contents in a few cases where it doesn't make sense, such as
for replaced elements. (I can dig up the exact conditions if you need it.)

So I think this change is basically wrong:
c65b958
rather than referring to literal display values, it would be better to use more general terms,
e.g. "when the element has a principal box, use its size, otherwise use the computed value"

@fantasai
Copy link
Collaborator

The CSSWG decided to treat display: contents as display: inline on replaced elements and form controls. Edited in c588376 by making it compute to inline on such elements. Please let me know if you'd rather do something more magical.

@zcorpan
Copy link
Member Author

zcorpan commented Jan 25, 2017

LGTM

@zcorpan zcorpan added Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. and removed Commenter Response Pending labels Jan 25, 2017
@MatsPalmgren
Copy link

I fully agree with bz in #1024 (comment)

The spec should say that contents computes to contents but the used value is inline for these elements.

In general, adding new spec language that makes a computed value depend on layout stuff such as box construction is pretty much always bad.

@tabatkins
Copy link
Member

WG discussed this today.

Problem with making it "behaves as" is that there are plenty of other properties that care about the box that the element will generate; it's generally very significant whether something generates an inline, block, or no box at all. So we can't just push this to used-value with the "behaves as" language; this is fundamentally a computed-value issue.

So impls either need to bite the bullet and allow this computed value to depend on form-control-ness (as, according to François, Firefox already does?), or shoot for a simpler resolution, like making display:contents suppress the box like normal (and, as a side-effect of being a form control, and thus half-replaced, also suppress its children's boxes, like display:none).

@tabatkins tabatkins reopened this Feb 22, 2017
@tabatkins tabatkins removed Closed Accepted by CSSWG Resolution Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. labels Feb 22, 2017
@emilio
Copy link
Collaborator

emilio commented Feb 23, 2017

Problem with making it "behaves as" is that there are plenty of other properties that care about the box that the element will generate; it's generally very significant whether something generates an inline, block, or no box at all. So we can't just push this to used-value with the "behaves as" language; this is fundamentally a computed-value issue.

While I see your point, knowing both Blink's and Gecko's implementation, this is sort of complex to implement at computed value time. All the logic to decide whether an element generates a box resides on the actual box construction step, probably with display: none as an exception. Gecko is rewriting the style engine to run ahead of box construction (not at the same time). Blink is doing a similar effort on its engine.

This will mean having to keep track of whether an element generates a box when its specified value is contents in two different places: at style resolution time, and at layout tree construction time, which requires significant changes in both engines as far as I can tell (specially in Gecko, for the way style sharing works there I think).

How painful or difficult those changes are I don't know ahead of time, tbh.

So impls either need to bite the bullet and allow this computed value to depend on form-control-ness (as, according to François, Firefox already does?).

Firefox doesn't. For a quick example, the following logs contents. The work in progress Blink implementation doesn't either (except for SVG elements, which I need to clean up depending on this resolution).

<!doctype html>
<button style="display: contents">Foo</button>
<script>
  console.log(getComputedStyle(document.querySelector('button')).display)
</script>

or shoot for a simpler resolution, like making display:contents suppress the box like normal (and, as a side-effect of being a form control, and thus half-replaced, also suppress its children's boxes, like display:none).

This is definitely simpler to implement, though I'm not sure all the people would be on board with this. @MatsPalmgren or @rune-opera may have opinions here?

@emilio
Copy link
Collaborator

emilio commented Feb 23, 2017

(Also please note that ATM I'm not an official voice from neither Blink or Gecko, I just happen to know a bit about how this is implemented in them)

@emilio
Copy link
Collaborator

emilio commented Feb 23, 2017

After a bit more thought, maybe even the display: none kind of handling isn't as easy as it seems, at least for elements that currently have custom layout boxes not always depending on display and may have arbitrary descendants, like <details>, <fieldset> and similar.

@FremyCompany
Copy link
Contributor

My comment on the call was that we had chosen display: contents to behave like display: inline instead of display: none because that was how Firefox layouted the controls (it also seemed to have the preference of most authors in the room).

I didn't check what the getComputedStyle function returned, but to be fair -- in Edge at least -- this function could return you any value independently of what we actually store in our formats, I don't think that is particularly relevant. If Gecko desires to store formats in one way that is not what a spec says the computed value is, it should be perfectly fine as long as there is some code in getComputedStyle that takes care of returning the expected value to authors, and they have function in their codebase like shouldComputedXyzabcBeAuto(style, boxSource) to adhere to existing spec text that conditions on if the computed value of Xyxabc is Auto.

In Edge, getting a value through getComputedStyle will regularly trigger some new cascade pass for the asked property if the value stored in the formats isn't enough to synthetize the value to return. This is desirable also to prevent :visited styles from leaking out for properties that might be affected by that.

In short, I am not entirely sure why there is so much push from gecko to "dumbify" the computed value of properties to the point they end up being the specified value in almost all the cases. The computed value is a concept, and is not tied to your implementation of formatting in any strong way.

@fantasai
Copy link
Collaborator

@tabatkins I'm not entirely sure that's the best summary. Afaict the conclusion was that it's a more complicated issue than expected and people needed more time to contemplate. It's on the agenda again for next week. Minutes are here: https://lists.w3.org/Archives/Public/www-style/2017Feb/0088.html

@FremyCompany At least one concern in Firefox is caching of computed values through inheritance or defaulting. There's more discussion about it in various old Flexbox threads on www-style.

@dbaron
Copy link
Member

dbaron commented Mar 1, 2017

The reason that the concept of computed value is important is that the computed value concept is what inheritance operates on. Inheritance happens as part of style computation.

Software architecture depends on being able to split software into pieces in useful ways. Being able to separate parts of software allows both maintainability and the ability to make the pieces happen at different times. While the divisions in different engines are somewhat different from each other, many performance optimizations that the Web depends on depend on separation between different parts of a software system in various ways. Currently style computation is tied to the differences between elements only (I think) via the UA stylesheet and via the rules for mapping presentational attributes into CSS, which are both forms of specified values that are the normal input to the style computation process. Likewise, style computation is separate from selector matching (which happens before it) and from box construction (which happens after it), which is in turn separate from intrinsic size calculation, which is separate from layout calculation, which is separate from painting and compositing.

Currently CSS computed values are a function of specified values (which include the UA style sheet, and are based on the results of selector matching), which pseudo-element the values are for (considering a regular element as a single type), and the computed values of the parent (i.e., what the element inherits from) and occasionally the grandparent. We've designed large amounts of software around those assumptions. While we might be willing to the substantial amounts of work to stop depending on these assumptions for a good reason, I don't want to change them for handling of an edge case that wasn't one of the actual use cases for which the feature was designed and implemented (and wasn't brought up until a year or so later).


I'd also like to point out that there's a third option: that for replaced elements, display:contents just removes the replaced-ness of the element and allows the children to display as though the replaced element weren't there. (The previous two options were treating as none, and treating as inline.) I think there's a decent argument that this might even be the most logical option.

@dbaron
Copy link
Member

dbaron commented Mar 1, 2017

The difficulty with my option 3 is that it's not necessarily clear what aspects of form controls are associated with their box and what aspects are associated with their DOM node. So it seems likely to lead to non-interoperability in terms of what aspects of the form-control-ness go away.

@dbaron
Copy link
Member

dbaron commented Mar 1, 2017

The group just discussed this, and ended up inclined towards display:none rather than trying to go through making display:contents show the children, for replaced elements and form controls.

In response to @emilio's #540 (comment) -- I don't quite follow the concerns, although it's not clear to me that the concern would be worse with one approach than the other (rather than the old display:inline, which has other serious problems). Though if you still think this is problematic, I'd be interested to understand better why it is.

@fantasai
Copy link
Collaborator

fantasai commented Mar 9, 2017

@bzbarsky raised the issue of what to do with <object>, which is technically a submittable form element, but is non-replaced when falling back, and also should probably be handled consistently with <video> in any case.

Options are:

  1. explicitly declare non-replaced fallback of replaced elements to be treated as display: none
  2. clarify that non-replaced fallback of replaced elements is still treated as non-replaced, and make <object> an exception to the form controls rule
  3. clarify that non-replaced fallback of replaced elements is still treated as non-replaced, and have <object> be inconsistent with how fallback is handled for <video> and <canvas>
  4. give up and try the “all of these things just display their DOM contents and lose their special rendering” option mentioned above

See minutes from the currently-active resolution on this issue here: https://lists.w3.org/Archives/Public/www-style/2017Mar/0001.html

@SelenIT
Copy link
Collaborator

SelenIT commented Mar 9, 2017

I am not sure that the button should be suppressed completely. I hoped that it could also display its content (stripping all rendering associated with the button itself, but preserving the activation behavior, somewhat similar to what a label element for a visually hidden form control does), and that would be a hack for making buttons behave as display:inline.

@tabatkins
Copy link
Member

We added a section with all the open questions to discuss.

@AmeliaBR
Copy link
Contributor

Thanks for the nice summary in the spec, of the current interpretations & open issues.

I do like the idea of using display: contents to un-replace replaced elements that also accept valid DOM children as accessible representations or fallback (<canvas>, <audio>, <video>, <meter>, <progress>), because I think that would be a nice feature to have.

But I would prefer that the replaced-element behavior is consistent with the shadow-DOM model of these replaced elements (that is, rendered as if the real DOM children are masked by a closed shadow DOM that is rendered in its place). I'm not sure I understand what that is: are the "contents" of a shadow DOM host the plain DOM contents, or the flattened shadow DOM contents?

If the "contents" of a shadow DOM host are the regular-DOM child nodes, then (IMHO) it makes sense that:

  • Void replaced elements (like <img> and <input>) effectively become display: none, since they have no true contents to display.
  • Replaced elements with valid DOM fallback children are treated as generic container elements with display: contents.

However, if the "contents" of a shadow DOM host are the flattened-tree contents, after slotting true children into the shadow tree, then display: contents wouldn't change which sub-elements are displayed, only their layout. The closest matching behavior for replaced elements would be display: inline (as previously spec'd).

Another was of looking at the same end result would be to treat display: contents as if the replaced element has no box, but the replaced content is always quarantined inside an anonymous inline box that holds its layout together (preventing an iframe from "bleeding into the parent document", for example).

@tabatkins
Copy link
Member

"display: contents" suppresses the box that the element would normally generate. Shadow DOM disappears by the time box-tree happens; the box tree is built based on the "flat tree", which has all shadows expanded out.

So yes, for a shadow host, "display: contents" just strips out the host wrapper, but all the contents are left exactly the same, just "hoisted up" one level in the tree.

We're pretty set on having replaced elements just go 'display:none' when you set 'display:contents', because they simply don't have any boxes below the replaced element's box; they're a black box as far as CSS is concerned. There's nothing to hoist!

Note that <video> and <audio> don't actually have "fallback contents" - they can contain normal DOM, but it's explicitly not a11y-related fallback, it's just there so you can show an error message to old clients that didn't support those elements. Again, tho, they're replaced - we don't care what their DOM content is, just what their box tree is, and they don't generate box-tree children at all.

@AmeliaBR
Copy link
Contributor

Thanks for the clarification, Tab.

I still think it's slightly confusing to have one layout rule for "native" shadow DOM (e.g., videos and complex form widgets) and another rule for custom web component shadow DOM, but that's consistent with them being very different from a DOM scripting perspective.

However, I would prefer to have a consistent rule for all the replaced elements with fallback content, whether that's an <object>, <video>, or a <progress>.

<canvas> is the odd one out, because it's child content isn't ignored when the canvas is rendered (it's still part of the accessibility tree & tab order), so it could have special rules.

@tabatkins
Copy link
Member

Again, <video> doesn't have fallback content; its children are never rendered or exposed in any way, in a browser that understands the element. So it's a different case from all the others.

I still think it's slightly confusing to have one layout rule for "native" shadow DOM (e.g., videos and complex form widgets) and another rule for custom web component shadow DOM, but that's consistent with them being very different from a DOM scripting perspective.

It's not even "one rule" for native shadows - <details> uses a native shadow, but it totally fine to just unwrap; it's not exotic in any way, and its box structure is consistent across browsers (it's just a shadow that pulls in the first summary child, then wraps all remaining content in a shadow div). Form controls have totally different box structures across browsers, tho, and those structures aren't exposed to CSS's box tree.

The rule is totally just "is there a reasonable box tree underneath the element, that can be exposed when we strip the wrapper".

All the replaced elements will get display:none regardless of whether they have fallback or not; the question is just what to do when their fallback triggers, and they stop being replaced.

@AmeliaBR
Copy link
Contributor

Again,

Couldn't you say the same about <progress> and <meter>?

The only elements that sometimes fall back and sometimes don't, in a given browser, are <object> (depending on data type) and <canvas> (depending on if JS is disabled).

@tabatkins
Copy link
Member

Couldn't you say the same about <progress> and <meter>?

No - the contents of progress/meter are fallback content, exposed to the a11y tree. They're meaningful in browsers that understand the element.

I think meter/progress are similar to br/wbr - they represent some text content, even if they're rendered as magical things. They should probably be display:none as a result, we just want to make sure the WG agrees.

The only elements that sometimes fall back and sometimes don't, in a given browser, are <object> (depending on data type) and <canvas> (depending on if JS is disabled).

And embed/applet, I think. (If they can't, then whatever, they just display:none all the time.)

@AmeliaBR
Copy link
Contributor

the contents of progress/meter are fallback content, exposed to the a11y tree

Not really, see: https://w3c.github.io/html-aam/#el-progress

Although there's still open debate about how best to expose meter as something different from progress, they are both expected to be exposed as atomic widgets, with the actual numerical properties mapped to the accessibility API equivalents. The text content wouldn't be exposed through the API in a supporting browser. It's only there for older browsers.

And embed/applet, I think.

Yes for applet (I had to look it up, it works like object). No for embed (which is void, no child/fallback).

But regardless, my main point was : ideally, try to be as consistent as possible, rather than special rules for each different element type. But I recognize that we don't live in an ideal world.

@astearns astearns removed the Agenda+ label May 1, 2017
@astearns
Copy link
Member

astearns commented May 2, 2017

We went through the issue list in the new section at the Tokyo meeting. If there's anything remaining to go over, please re-add the agenda+ tag.

@tabatkins
Copy link
Member

We edited in all the f2f resolutions.

Please let us know if this is satisfactory, or if further work is required. A response would be appreciated @zcorpan @AmeliaBR @bzbarsky

@bzbarsky
Copy link

Delegating to @MatsPalmgren.

@zcorpan
Copy link
Member Author

zcorpan commented May 16, 2017

I discussed with @rune-opera, at first we thought object and img could show the children when they're not replaced elements. But object loading depends on the style of the element -- it doesn't load for display:none. So then it could be a new race condition (if the style is async-loaded in some manner, which I believe is something web devs do these days) and be pretty confusing for object, if it ends up as display:none or rendering the children. So always display:none does seem better.

So LGTM for resolving this bug. Further work could include upstreaming some of the logic to HTML, writing web-platform-tests, filing browser bugs... 😊

@bzbarsky
Copy link

io then it could be a new race condition (if the style is async-loaded in some manner

@zcorpan I'm not sure I follow. What sort of style are you envisioning async-loading?

@zcorpan
Copy link
Member Author

zcorpan commented May 16, 2017

I mean something like https://github.com/filamentgroup/loadCSS

@bzbarsky
Copy link

Yes, I understand what async-loading is. What I'm not sure is what you think is racing against what. As in, what specific rules are being async-loaded, and what does the DOM look like?

@zcorpan
Copy link
Member Author

zcorpan commented May 16, 2017

Oh, ok.

If the behavior of display: contents were to act like display: none when object is a replaced element but render the children when object is showing fallback, then the end result would depend on which happens first: loading of the resource vs. applying the style. If it's first loaded, then the end result would be display: none. If the style is applied first, then the object element doesn't generate a box, so it's not "being rendered", and thus doesn't load anything, but continues to render the children.

http://software.hixie.ch/utilities/js/live-dom-viewer/saved/5186 for object first
http://software.hixie.ch/utilities/js/live-dom-viewer/saved/5187 for style first

@bzbarsky
Copy link

Hmm. Do UAs actually implement that "being rendered" requirement across the board? I'm pretty sure Gecko doesn't unless the type is a plug-in type (and even then, I think it may only affect plug-in instantiation, not the actual load, since the type is not known until you do the load), so both your testcases render the image in Gecko.

Per the letter of the spec, every time an , or any ancestor, gets toggled to display:none and back it should redo the fetch for its data attribute, which seems like a really weird behavior. I filed whatwg/html#2692

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests