Skip to content

[css-content] Implementing content:none on elements is not web-compatible #6503

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

Closed
MatsPalmgren opened this issue Aug 9, 2021 · 14 comments
Closed

Comments

@MatsPalmgren
Copy link

The current spec says:

none
On elements, this inhibits the children of the element from being rendered as children of this element, as if the element was empty.

We implemented that in Gecko (bug 1699964) but sadly it broke a bunch of sites so we had to disable it for now (bug 1719239).

So the spec needs to say that none has no effect on elements (what all UAs currently implement AFAIK).
If we want to retain this feature, then we need a new keyword for it (children-none or whatever).

CC @emilio

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed content:none, and agreed to the following:

  • RESOLVED: spec content:none as having no effect on elements
The full IRC log of that discussion <fantasai> Topic: content:none
<Rossen_> github: https://github.com//issues/6503
<fantasai> emilio: We implemented it, broke tons of sites
<fantasai> emilio: We should at least fix the spec to say that content:none has no effect on non-pseudo elements
<fantasai> emilio: It's a bummer because 'content' does have effects on some elements
<fantasai> emilio: We could add another value to the content property to represent that behavior
<fantasai> Rossen_: how broken?
<fantasai> emilio: content:none suppresses boxes for children of element
<fantasai> emilio: sites were specifying it on a bunch of stuff using e.g. * { content:none; }
<fantasai> emilio: initial value is normal
<fantasai> florian: I wouldn't have expected it to be Web-compatible, has been a no-op for a long time, so mistakenly applied in lots of places
<fantasai> emilio: any objection to change the spec to match reality?
<fantasai> emilio: other question is should we add a new value
<Rossen_> q?
<Rossen_> ack fantasai
<florian> fantasai: we should spec it
<florian> fantasai: if we need a new keyword, I'd suggest "empty"
<florian> fantasai: not too sure what the use cases are though
<fantasai> Rossen_: any other comments?
<fantasai> emilio: not sure we should even add the keyword, we implemented for completeness
<rachelandrew> it would be interesting to see if authors have use cases
<fantasai> ??: I think there could be some interesting cases for having the box but not its contents
<bradk> That was me
<astearns> s/??/bradk/
<TabAtkins> we can just leave the discussion out of this issue for now, and open a fresh issue arguing for adding this new keyword
<fantasai> emilio: also interesting question about interaction with content-visibility:hidden
<TabAtkins> no reason to tie it to this discussion
<astearns> +1 to new issue for use cases
<fantasai> bradk: maybe ask authors
<emilio> +1
<fantasai> Rossen_: ok, let's deal with that in a separate issue
<fantasai> RESOLVED: spec content:none as having no effect on elements

@dholbert
Copy link
Member

dholbert commented Aug 11, 2021

Just to clarify what "having no effect" is going to mean here: it looks like in both Firefox and Chromium, content:none just computes to normal. Is that what we want to spec here? (i.e. are we saying none is effectively an alias for normal, and so it computes to normal) Or is this going to be something more subtle?

(For completeness: I tested WebKit as well, but their getComputedStyle implementation seems to just return the empty string for the computed value of content, when the specified value is normal or none, so I'm not as sure what's going on there.)

@MatsPalmgren
Copy link
Author

Gecko resolves none to normal for elements but still computes it to none, i.e. it inherits as none. For example:

<style>
li { content: none }
li::marker { content: inherit }
</style>

<ul><li>x</ul>

<script>
let li = document.querySelector('li');
li.textContent = getComputedStyle(li).content + ' ' +
                 getComputedStyle(li, '::marker').content;
</script>

Firefox: shows "normal none", and no bullet since ::marker inherited none (note that we did this even before implementing content:none for elements so there was no change in this regard from those patches)
Chrome: shows "normal normal" , and a bullet

I think we need to sort this out while we're here since it might be a web-compat issue too...
See also similar issues: #1757 #821 #4632

@Loirooriol
Copy link
Contributor

Just to clarify Chromium's behavior. Chromium implements content as a single linked list. Both normal and none used to be represented as nullptr (empty list). So they were equivalent. When serializing a computed style, nullptr would become none for ::before and ::after, and normal for other things.

But I changed the behavior when implementing ::marker. Now empty list means normal, and none has its own value. So they now have different computed values. But I didn't want to have compat problems, so I kept the old serialization behavior for things other than ::marker.

Regarding the inherit example, ::marker fails to inherit none because content: inherit never works in Chromium:

void Content::ApplyInherit(StyleResolverState& state) const {
  // FIXME: In CSS3, it will be possible to inherit content. In CSS2 it is not.
  // This note is a reminder that eventually "inherit" needs to be supported.
}

@w3c w3c deleted a comment from ellenmartin22 Nov 17, 2021
@dholbert
Copy link
Member

[note: to reduce confusion/noise, I've just deleted a drive-by comment which was just a copypaste of the initial comment here with nothing additional added. The poster seems to have posted similar comments in other repos as well; not sure if they're a bot or just testing out github, but in any case it wasn't adding information here.]

@frivoal
Copy link
Collaborator

frivoal commented Dec 22, 2021

Agenda+ to decide if we want to make none compute to normal on regular elements, or keep them as distinct values with identical behavior.

@dholbert
Copy link
Member

dholbert commented Mar 29, 2022

Agenda+ to decide if we want to make none compute to normal on regular elements, or keep them as distinct values with identical behavior.

[tl;dr: we may already be interoperable on them being distinct values, so maybe we should stick with that?]

Looking at the two most recent contentful comments here, it seems like Gecko and Chromium both have these keywords implemented as distinct computed values right now. This just wasn't easily-observable in Chromium, because as @Loirooriol said, content:inherit is nerfed in Chromium (which is a bug/TODO to be eventually fixed, and IIUC that's why Chromium disagrees with Gecko on Mats' testcase from his comment above.)

(I also tested WebKit using Mats' testcase from his comment above but it just renders with a single bullet, so I'm not sure they have this implemented at all right now... or maybe they just don't include content in the output of getComputedStyle? Not sure.)

So I think maybe the existing implementations in Chromium and Firefox at least are interoperable on these keywords being distinct computed values, aside from that Chromium bug about content never inheriting which makes it harder to observe the interoperability. Do I have that correct?

(@Loirooriol , please correct me if I'm misunderstanding your comment.)

@Loirooriol
Copy link
Contributor

That's correct, they are different computed values in Blink, but I don't think that's observable other than in ::marker, since content: inherit doesn't work, and getComputedStyle lies.

Regarding WebKit, note that ::marker doesn't obey content at all. But anyways, normal and none are also different computed values, since none sets the hasEffectiveContentNone() flag. Like in Blink, content: inherit doesn't work. But WebKit has something interesting: getComputedStyle doesn't lie, i.e. it can be normal or none, whatever you specified. The only exception is that content: normal resolves to none on ::before and ::after.

So this seems to imply that it's web compatible if elements keep content: none as-is in getComputedStyle. Though it was a recent change, so maybe there hasn't been enough time to encounter compat problems.

@dholbert
Copy link
Member

dholbert commented Apr 5, 2022

Thanks. It sounds like Gecko/Blink/WebKit all represent 'normal' and 'none' as distinct computed values. So in response to @frivoal 's agenda+ comment...

Agenda+ to decide if we want to make none compute to normal on regular elements, or keep them as distinct values with identical behavior

Of these two options^, we probably want the latter option (distinct computed values), so as not to unnecessarily force all engines to change (unless there's good reason to do so).

Having said that: there's some additional magic and/or bugs in how the value is used/not-used in each engine, so I'm not sure what makes to spec about that.

@dholbert
Copy link
Member

dholbert commented Apr 6, 2022

So I think the only question here is whether we want to update the resolution above (spec content:none as having no effect on elements) to additionally spec that content:none resolves to normal as part of having-no-effect.

CC @graouts who implemented the recent WebKit commit that keeps it distinct (according to @Loirooriol above), in case they have opinions here, since they are alone on that keeping-the-resolved-values-distinct behavior for the moment (but that is nice & straightforward if it's web-compatible).

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-content] Implementing content:none on elements is not web-compatible, and agreed to the following:

  • RESOLVED: none does not compute to normal
The full IRC log of that discussion <dael> Topic: [css-content] Implementing content:none on elements is not web-compatible
<dael> github: https://github.com//issues/6503
<dael> dholbert: Basically we arrived at a point where wondering if content:none and normal should be 2 alues as computed style or if it should be alias
<dael> dholbert: Seems everyone impl as 2 values. I think that's new information
<dael> dholbert: I think we want to stick with that unless reason to change
<dael> dholbert: Some subtilty around how none resolves to normal and browsers disagree a bit but I think it's mostly known bugs.
<florian> q+
<dael> dholbert: I think this issue is no change, but not 100% sure. oriol has done more investigation and left useful comments. Curious his thoughts
<dael> oriol: The thing is that in Blink while content:noen and normal are diff computed values you cannot observe it from webpages. One resolves to the other. Possible this could change. When I impl none I didn't want to risk compat but I didn't try.
<dael> oriol: In WK there's a recent change from Jan where if set content:none gCS provices none. Otherside get normal. So get diff in WK and they didn't have to revert so seems it's web compat
<dael> oriol: I think FF impl and that's web exposed. So maybe can consider different and say resolve to themselves
<astearns> ack florian
<dael> florian: I think historically the 2 main contexts were regular elements where content:none did nothing and before and after pseudo. marker forces difference where normal isn't empty but none is.
<dael> florian: From there I think should try for as distinct as we can. if we have compat issues we can do to the extent required but should keep separate by default
<dael> astearns: dholbert you suggested close no change?
<dael> dholbert: I had forgotten what oriol mentioned. I tend to think we should wait to see how the WK change survives. If they can keep as distinct in gCS that's the simpliest and I'm sure easy change for Gecko and Blink
<dael> dholbert: Potentially resolve in favor of that but might be premature since WK has recently shipped so I don't know how much exposure it has got
<florian> [seems reasonable to me]
<dael> astearns: Make no resolution and leave issue open for now?
<dael> dholbert: Yeah. Might re-agenda+ for another question. i think original request to make them compute to same I think have decided we don't need to. now question of if we can keep them distinct
<florian> s/before and after pseudo/before and after pseudo where "normal" was empty/
<dael> dholbert: I can add a summary on the issue
<dael> astearns: Could resolve we're not going to have them compute to each other
<dael> dholbert: Happy on that since it matches existing impl
<dael> astearns: Prop: none does not compute to normal
<dael> astearns: Will leave issue open with summary from dholbert on what we need to see with time
<dael> astearns: Obj?
<dael> RESOLVED: none does not compute to normal

@dholbert
Copy link
Member

dholbert commented Apr 6, 2022

Summarizing the CSSWG discussion:

We'd like to circle back to this after https://bugs.webkit.org/show_bug.cgi?id=235222 has gotten some more testing. If I'm understanding @Loirooriol's summary correctly, that change makes WebKit's getComputedStyle truthfully report the actual computed of content, without any "resolve-none-to-normal" magic. (Though none still behaves as normal in many situations; i.e. the used value of none may still be normal.)

If that seems to be web-compatible & WebKit is able to successfully ship that change, then it seems like we should spec that behavior and change Gecko/Blink to match, to remove unnecessary magic & make getComputedStyle a bit closer to reporting the actual computed style (rather than the used style).

@SebastianZ
Copy link
Contributor

Though none still behaves as normal in many situations; i.e. the used value of none may still be normal.

This is the important point here. For elements, the used value needs to be normal. The examples of broken pages linked to from the Mozilla bug all had in common that they accidentally hide normal elements with content: none while they only wanted to empty the pseudo-elements ::before and ::after or not empty the content at all.

Sebastian

Sebastian

@Loirooriol
Copy link
Contributor

@SebastianZ The resolution was making none compute as-is. On elements, it will still behave like normal.

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

8 participants