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-pseudo] Which properties to reset in ::marker #4568

Open
faceless2 opened this issue Dec 6, 2019 · 28 comments
Open

[css-pseudo] Which properties to reset in ::marker #4568

faceless2 opened this issue Dec 6, 2019 · 28 comments

Comments

@faceless2
Copy link

faceless2 commented Dec 6, 2019

Migrated from #4474

The specification currently states that only the following properties apply to ::marker:

  • all font properties (see [CSS3-FONTS])
  • the white-space property (see [CSS3TEXT])
  • the color property (see [CSS3COLOR])
  • the text-combine-upright, unicode-bidi, and direction properties (see [CSS-WRITING-MODES-3])
  • the content property (see [CSS-LISTS-3])

...and that this list is likely to be revised. But markers don't exist in a vacuum; they're part of the tree, so even properties that don't apply will need to be reset if they're inheritable, to initial or some other sensible value.

As of today the specification has

unicode-bidi: isolate;
font-variant-numeric: tabular-nums;

with white-space to be reset to something closely resembling pre (#4448).

So, starting with inheritable properties in css-text and text-shadow from css-text-decor, and working on the presumption that markers will always be limited to a single line, we have these properties to consider:

propertyapplicability to markers
hanging-punctuationprobably n/a
hyphensn/a
letter-spacing?
line-breakn/a
overflow-wrapn/a
tab-sizetabs in markers?
text-align-*?
text-indent?
text-justifywill depend on text-align
text-transform?
white-spacealready covered
word-breakn/a
word-spacing?
word-wrapn/a
word-wrapn/a
text-shadow?

I'm going to leave tab-size, it's technically applicable but the only place this would ever happen is a WPT test-case. I suppose a decision has to be made, but I doubt it matters what that is.

I think hanging-punctuation doesn't apply - all values other than force-end will apply only if the text wraps, and force-end only applies to the end of the line. As white-space won't collapse in markers, and a space will almost always follow the punctuation I don't think that's the case, but will test for completeness (although as this depends on browser support for both hanging-punctuation and custom markers, it might not be a very useful test).

Test is here: https://output.jsbin.com/rusilefogo

Findings:

propertyFirefoxChromeSafari
letter-spacingNYY
word-spacingNYY
text-indentNNN
text-alignYNN
text-transformNYN
text-shadowNYN
hanging-punctuationN--

Everyone's different, isn't that fun! To start the ball off on the discussion below, I'd suggest:

  • letter-spacing - arguable either way.
  • word-spacing - arguable either way but I lean towards no - as white-space will separate the marker from the content, word-spacing will always be applied and I don't think that's going to be compatible with what users expect.
  • text-indent - surely no. Every browsers says no.
  • text-align - I think this has to be a no, the FireFox results are clearly wrong
  • text-transform - arguable either way; if it wasn't for the kana transformations I would say no, but I'm not sure how applicable they are to markers so can't comment
  • text-shadow - arguable either way but I lean towards yes - I think the result in Chrome is more consistent visually.
  • hanging-punctuation - if it applies at all, resetting to initial seems a good idea and matches current behaviour in Firefox. It's not testable in Chrome or Safari, as we need to remove the trailing space from the marker content, which I can't do without ::marker/content or @counter-style support.

(edit: pinging @MatsPalmgren @dauwhe and @fantasai for input as suggested by @fantasai in the original issue)

@Loirooriol
Copy link
Contributor

There is also #4206 about text-transform.
Note that not even Chromium legacy and Chromium LayoutNG are interoperable.

text-indent - surely no. Every browsers says no.

Actually, Chromium LayoutNG supports it, but you don't see it because of the marker alignment, so you need ::marker { direction: rtl }. And Firefox supports it in content markers but not in legacy markers (nsBulletFrame).

text-align - I think this has to be a no

Note Chromium LayoutNG does support it, but maybe not like you expected? If you use content: 'ab\a c' and white-space: pre, with text-align: left the c appears below the a, and with text-align: right it appears below the b.

In general I would lean towards not resetting properties unless strictly necessary.

@Loirooriol Loirooriol added the css-pseudo-4 Current Work label Dec 6, 2019
@faceless2
Copy link
Author

Whoops, I should have qualified that I'm testing with the versions I have here, which are the latest public ones available - your view from inside is much more accurate, thank you. I presume you have edit rights, if you want to edit that table to correct it please be my guest.

I'm making the presumption that markers will not contain newlines. As I said in #4448 I think multi-line markers are a can-of-worms - if they're allowed, the list above will certainly need updating.

@fantasai
Copy link
Collaborator

fantasai commented Dec 6, 2019

so even properties that don't apply will need to be reset if they're inheritable

So... I think this is an argument for clearly defining which properties apply to raw text vs to boxes vs to line boxes or their interaction with text, and clarifying that you can set declarations on ::marker and they will inherit to the text it contains. (Applying a property to a box vs letting it inherit through and affect its contents are slightly different concepts.)

I have no problem with allowing properties that affect text and not boxes to be applied to ::marker and to inherit through, since that wouldn't change behavior as we further develop a layout model for marker boxes.

I am concerned about things like text-align and text-indent since they reference the edges of the line box, and therefore their behavior could change as we define the layout model for marker boxes. From my perspective, setting text-align: end !important; text-indent: 0 !important; hanging-punctuation: none !important in the UA style sheet would be appropriate at this point in time (and at the point where layout is clearly defined, we can drop the !important.).

For the rest, I would let text-shadow inherit from the list item element the same as color. I suspect it's better to block inheritance of text-transform by default via text-transform: none, since casing is often used to distinguish list items. Wrt resetting word-spacing and letter-spacing I'm unsure. We have to remember that this resetting would be applied to inside markers as well as outside ones, and for inside markers in particular might make sense to inherit the paragraph's conventions.

So I guess my proposal would be:

  • Allow properties that style text directly to ::marker, i.e. add word-spacing, letter-spacing, text-shadow, and maybe text-emphasis (if UAs want to implement it) alongside the font properties; they all effectively just inherit through to the text rather than applying to ::marker.
  • Add text-align: end !important; text-indent: 0 !important; hanging-punctuation: none !important to the UA style sheet of any browser that supports those properties on ::marker.
  • Add text-transform: none to the UA rule for ::marker.
  • Don't add word-spacing or letter-spacing resets to the UA rule for ::marker.

I'm not entirely sure about the last two. @dauwhe ?

@faceless2
Copy link
Author

We have to remember that this resetting would be applied to inside markers as well as outside ones,

The inability to distinguish between inside and outside markers is consistently making the process of styling them (both in terms which properties apply, and what their defaults are) quite a bit more complicated. Would something like ::marker:inside and ::marker:outside be worth considering?

@Loirooriol
Copy link
Contributor

We should also consider text breaking properties. For example, it's not clear if overflow-wrap may affect outside markers, but definitely it can affect inline ones. As usual,

  • Firefox obeys inherited overflow-wrap when ::marker has a non-normal content. Otherwise the marker is unbreakable.
  • Chromium obeys inherited overflow-wrap with LayoutNG. Otherwise the marker is unbreakable.

@frivoal
Copy link
Collaborator

frivoal commented Dec 9, 2019

The inability to distinguish between inside and outside markers is consistently making the process of styling them (both in terms which properties apply, and what their defaults are) quite a bit more complicated. Would something like ::marker:inside and ::marker:outside be worth considering?

I suspect you're right, and that we need something like this. Whether we actually expose it is an interesting question, but given that inside markers and outside markers seem call for different behaviors, effectively that's the model we're using. Given that this doesn't introduce any selector-styling loop shenanigans, I suspect we should make it explicit and expose it.

And from there, it looks like quite a few of the resets (with or without !important) should be on ::marker:outside rather than on ::maker, as the inside case seems to need much less magic.

@Loirooriol
Copy link
Contributor

Loirooriol commented Dec 9, 2019

Note that adding :inside and :outside, especially if they are exposed, would close the door to supporting list-style-position in ::marker in the future, since it could introduce a circularity. https://wiki.csswg.org/faq#selectors-that-depend-on-layout

If display: marker or position: marker are reintroduced, these properties could be affected too.

@frivoal
Copy link
Collaborator

frivoal commented Dec 9, 2019

Not really, since list-style-position needs to be applied to the list item to take effect, not the marker. So there's no loop. Applying list-style-position on the ::marker would work just fine if the marker itself was a list item, and then it would change the position of ::marker::marker.

If we don't allow having markers on markers, then it is only natural than applying list-style-position on the marker itself would do nothing.

@Loirooriol
Copy link
Contributor

Sure, I mean that for example Chromium legacy reads the list-style-position of the marker itself (LayoutListMarker) instead of the list item. That's a valid interpretation because list-style-position inherits and is not syntactically supported in ::marker. And it's not unreasonable for the ::marker to obey its own list-style-position instead of the list item one. This interpretation won't be possible once we allow all properties in ::marker if we have :inside and :outside.

@Loirooriol
Copy link
Contributor

Exposing :inside/:outside could make :has() impossible, or at least prevent it from containing pseudo-elements

:has(::marker:outside) { list-style-position: inside }

@faceless2
Copy link
Author

As in, that selector would result in the CSS oscillating between two states? True, but I can also think of:

:not(:has(::marker)) { display: list-item }
:has(::marker) { display: block }

I can't get :has(::marker) to match at all, so I've not idea if this sort of loop is already covered. Preventing it is obviously important, but I don't think it's a problem specific to :inside / :outside.

@Loirooriol
Copy link
Contributor

Loirooriol commented Dec 11, 2019

@faceless2 There is no circularity with ::marker alone, since it exists even with display: block. See #1793 (comment)

RESOLVED: marker exists on all elements, on ::before and on ::after but no box unless it's display: list-item

@faceless2
Copy link
Author

faceless2 commented Dec 11, 2019

Ah, very true. I'll try to come up with another one :-)

Edit: not sure I can actually. So yes, I agree that setting list-style-position as in your example would cause an issue, and it's probably a new one. Whether it's enough on its own to rule out :inside / :outside is the question.

@frivoal
Copy link
Collaborator

frivoal commented Jan 7, 2020

While we're considering which properties apply or don't, and reset or don't, it seems that the text-decoration properties in general, and the text-decoration-skip family of properties in particular should apply. (initially raised in #843).

@Loirooriol
Copy link
Contributor

#4574 also has some discussions about text decoration in markers.

@fantasai
Copy link
Collaborator

fantasai commented Jul 9, 2020

Filed follow-up to #4568 (comment) in #5303

@Loirooriol
Copy link
Contributor

Agenda+ F2F because resetting text-indent is even encouraged in CSS Text

Since the text-indent property inherits, when specified on a block element, it will affect descendant inline-block elements. For this reason, it is often wise to specify text-indent: 0 on elements that are specified display: inline-block.

And I guess text-align: end might be useful for multiline outside markers? Firefox is already setting that.

No strong opinion about the others, but I don't see much benefit in resetting them.

@fantasai
Copy link
Collaborator

fantasai commented Jul 27, 2020

@Loirooriol My position is in #4568 (comment) Basically:

  • We allow any properties that apply to text to inherit through to the text.
  • We disallow or enforce any properties that apply to the box or the line box.
  • We reset text-transform as we resolved earlier.
  • Unsure about word-spacing and letter-spacing.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-pseudo] Which properties to reset in ::marker, and agreed to the following:

  • RESOLVED: If property applies to text and no dependency on box geometry it can be set on ::marker and inherits to text of marker
  • RESOLVED: Other properties which are not explicitly listed as apply to ::marker must not have an effect when set by author. UA might treat as not applying or treat them as set at UA important level but either way author should not be able to effect rendering
The full IRC log of that discussion <dael> Topic: [css-pseudo] Which properties to reset in ::marker
<dael> github: https://github.com//issues/4568
<dael> oriol: Related to discussion 2 weeks ago where resolved text-transform applies to ::marker and set to none by default.
<dael> oriol: Same for other properties
<dael> oriol: Text-indent property. Seems clear inheriting seems bad
<dael> oriol: Only applies to block container so can effect markers with outside position
<dael> oriol: May not be obvious to authors b/c they're not explicitly setting a display value to generate block. It's done by UA
<dael> oriol: Should follow rec in Tables which says if you set display-inline:block to element you reset text-indent to 0. Since UA gives inline block behavior to outside markers makes sense to reset text-indent to 0
<dael> oriol: fantasai went further and prop to do so with an important flag so until we have clear model of outside marker layout we prevent authors from changing.
<dael> oriol: Also works for me.
<fantasai> https://drafts.csswg.org/css-lists-3/#marker-properties
<dael> oriol: There are other properties but let's discuss this
<Rossen_> q
<Rossen_> ack fantasai
<dael> fantasai: We have a short list of properties that apply to ::marker and makes sense to be clear what we're talking about. Properties that apply to box/linbox and htose that apply to text. Not clear distinguishing
<dael> fantasai: Any property apply to text should inherit through ::marker and apply to text
<dael> fantasai: Those that apply to box or linebox b/c we dont have a layout model these properties should not apply. If they do apply they should be forced to initial value
<dael> fantasai: When we have a proper box model we can make those that make sense apply.
<dael> fantasai: I would change to properties that apply to ::marker and say then list properties that inherit to text and say you can set on ::marker, don't apply there but effect contents. If we take that as principle these questions become more obvious
<fantasai> s/effect/affect/
<dael> AmeliaBR: You argue color doesn't effect marker as a special thing it effect anon text box
<dael> fantasai: yeah
<fantasai> s/effect/affect/
<dael> AmeliaBR: If it effect inline text spans you can set on ::marker and it goes through. Anything for layout box has to be on allow list
<dael> fantasai: I think that's the principle. There's fun with fill and stroke where rely on geometry of box
<dael> AmeliaBR: We don't have impl of that
<dael> fantasai: Fair but still got to be careful
<dael> AmeliaBR: At some point will need to be defined
<dael> fantasai: Prop: Properties that apply tot ext with no dependency on geometry of boxes can be set on ::marker and inherit through to text
<dael> fantasai: Properties that apply to boxes and line boxes do not apply yet
<dael> oriol: But text-align FF sets to end so if you have multi line in outside marker with different widths they will be aligned to r in ltr
<dael> fantasai: text-align doesn't apply to text, it appleis to line boxes. So it would not apply to ::marker. Position of text I believe is undefined. If an impl supports text-align that's fine but need to make sure it's not changable by author such that they can get different results
<faceless2_> q+
<dael> fantasai: Either done through magic or through text-align but forced to that value and can't be changed by author. Once there's a layout model we loosen the restriction
<dael> faceless2_: What about when have replaced content with ::marker we have to propagate properties which allow you to resize
<Rossen_> ack faceless2_
<dael> fantasai: Replaced content is replaced content that's a child of the box. For list-style:image there's sizing rules that are particular ans size the image to 1em if it doesn't have a size, something like that. Would continue to apply
<dael> fantasai: If we want to do something special for images that are list markers we can think about that.
<dael> fantasai: Not sure
<dael> faceless2_: I think was difference between setting content and setting string of content. Bit scratchy on terms. Does seem to vary across impl but a lot of print lets you use image and content. Height is prop to image rather than box.
<dael> faceless2_: Agree in general with you
<dael> fantasai: ON marker set content to URL that's an image. THen marker is replaced element and width and height applies
<emilio> q+
<dael> faceless2_: Yeah, I think that's how it works with marker-content:url Does vary cross impl
<dael> TabAtkins: Yeah, WK based treat the pseudo as replaced. Spec requires anon element.
<dbaron> There was a spec draft of css3-content at some point defining that behavior.
<dael> AmeliaBR: But it is something to define to be similar to how list style with an image would work to make ::marker case more logical if can have it same as content with image. BUt neither is well defined
<fantasai> yeah, and I think there was some interesting questions around web-compat... but if WebKit is able to ship that
<Rossen_> ack emilio
<dael> emilio: I think Gecko always wraps it in an inline. But content URL on element impl disagree
<Rossen_> ack fantasai
<dael> fantasai: I think we should dig more into content url becoming replaced, but let's file a sep issue
<faceless2_> +1 to that.
<dael> Rossen_: With that issue are we ready to resolve?
<dael> Rossen_: I see heads nodding
<dael> Rossen_: fantasai can you give us prop ideally with a list of properties or a term of art?
<dael> fantasai: I think that's something we need to clean up. Two approaches is we inline...the entire fonts spec ans this set of text.
<dael> fantasai: We can also audit all our properties and spec in the applies to line.
<emilio> fantasai: https://github.com//issues/2889 is open for `content: url()` already, fyi
<dael> fantasai: For this I think the prop will be 2. 1) If property applies to text and no dependency on box geo it can be set on ::marker and inherits to text of marker
<dael> Rossen_: Similar to set of properties for ::first-letter?
<dael> fantasai: No b/c can take initial-letter
<dael> AmeliaBR: Might be to selection or highlight
<dael> fantasai: No, they can't change layout
<dael> fantasai: Might be first-line similar
<dael> fantasai: Rule is if property applies directly to text and no dependency on geo you can set it on ::marker and it will inherit to text
<fantasai> s/geo/box geometry/
<dael> Rossen_: Thoughts or objections?
<dael> heycam: Will we define in a ua stylesheet in a spec or make it clear what computed style is?
<dael> fantasai: Compute on ::marker and inherit through
<dael> heycam: So it's about if they apply
<dbaron> There might be two parts to this resolution: what we want to happen, and at what level the spec needs to define it?
<dael> heycam: Does that mean text-align:end is not correct?
<dael> fantasai: That's second resolution
<dael> Rossen_: Objections to If property applies to text and no dependency on box geo it can be set on ::marker and inherits to text of marker
<dael> RESOLVED: If property applies to text and no dependency on box geometry it can be set on ::marker and inherits to text of marker
<emilio> faceless2_: I don't see the webkit / blink quirk you mentioned in http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=8312 fwiw. All browsers behave the same
<dael> fantasai: Second If a property that's not in the first category is not listed in the spec than it does not apply to ::marker boxes. If an impl is taking advantage of infrastructre to handle marker layout it needs to bake that in as !important so author can't set
<dael> fantasai: Ex we spec that unicode bidi does apply to ::marker. It can be set. We didn't spec that text-indent does. Gecko uses text-align to position text and that's fine but it needs to set !important on that rule so that author can't set it and et different results.
<dael> fantasai: When we define the box model we can release that restriction. Until then we need to make these so author cannot effect
<fantasai> s/text-indent/text-align/
<dael> oriol: All inherited properties except the ones in the resolution should be set to initial using !important?
<dael> fantasai: I don't know that...are there prop for the box that inherit?
<dael> oriol: text-align chromium just inherits. Should we allow this?
<dael> fantasai: If a property has an effect in an impl and we say it does not apply the property needs to be force set so it acts like it does not apply
<dael> dbaron: Differences are observable in computed
<dael> fantasai: Yeah. Ultimate goal is they do apply but we need to define marker box model for that
<dael> Rossen_: Hopefully not paint in corner
<dael> fantasai: I don't think we will. Force set will be UA initial values. Author might set it on ancestor and we need to make sure it doesn't inherit through in unexpected ways
<faceless2_> @emilio If you set a height on that image, the spec says it should be "content-replacement" and the width/height should apply _to the image_. This is distinct from a list of more than one item, when it becomes a "content-list" - the distinction tab was making. That distinction is not made by the browsers, but if you're going to do ::marker { content: url(file.svg); height: 1em } that's the only way you can adjust the size of the SVG.
<dael> fantasai: Prop: Other properties which are not explicitly listed as apply to ::marker must not have an effect when set by author. UA might treat as not applying or treat as set at UA important but either way author should not be able to effect rendering
<dael> oriol: May or must on inheritence allowing?
<dael> fantasai: Must. Must behave as no effect
<dael> Rossen_: Custom properites as well?
<dael> fantasai: They don't effect layout, no
<dael> Rossen_: Those that can be used by custom layout?
<dael> fantasai: I don't think it effects ::marker
<faceless2_> @emilio https://github.com//issues/4632
<dael> Rossen_: I can have it inside and prop a bunch of properties. If you stop from prop to me I can't do custom layout
<dael> fantasai: Can't do it without display property and it does not apply to ::marker or things within it
<dael> Rossen_: Same with custom paint?
<dael> AmeliaBR: We can't do b-g image b/c that depends on layout. So for now probably
<dael> Rossen_: Making sure we're not in a corner
<dael> AmeliaBR: Any reason to prevent custom properties from having a proper computed value on ::marker? No reason to not let read in JS
<dael> fantasai: Only properties effected are notse not in the previous resolution and not listed in spec but would have effect in impl for some reason. Custom properties don't fit into any of those caregories
<emilio> faceless2_: uhhh, so the spec doesn't match any browser implementation of pseudo-elements? fun!
<dael> Rossen_: Objections to Other properties which are not explicitly listed as apply to ::marker must not have an effect when set by author. UA might treat as not applying or treat them as set at UA important level but either way author should not be able to effect rendering
<faceless2_> Matches at least two print engines though :-)
<dael> oriol: Clarification. THe might in the prop text should be a must
<heycam> (it also is impossible to stop custom properties from inheriting by setting things in the UA sheet, since all doesn't target them)
<fantasai> s/effect/affect/
<dael> RESOLVED: Other properties which are not explicitly listed as apply to ::marker must not have an effect when set by author. UA might treat as not applying or treat them as set at UA important level but either way author should not be able to effect rendering
<fantasai> s/might treat/MUST treat/
<dael> dbaron: Back to previous resolution.
<dael> dbaron: Thing that might still make people unhappy is not substance but level of detail where spec desc.
<dael> dbaron: Best thing to do is have editor apply it and bring back to group to see if we're okay with level of detail. There's a wide range of ways to do that. Seem reasonable?
<dael> AmeliaBR: Qualify our resolution that spec text is subject to final approval?
<dael> Rossen_: We can always amend a resolution
<dael> Rossen_: dbaron comments will go in the issue

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 28, 2020
The CSSWG resolved in w3c/csswg-drafts#4568
that properties like 'text-indent' that don't apply to ::marker, should
not be able to affect the ::marker via inheritance when set to an
ancestor.

Therefore, this patch sets 'text-indent: 0 !important' in UA origin.

Bug: 1031667

TEST=external/wpt/css/css-pseudo/marker-content-023.html
TEST=external/wpt/css/css-pseudo/marker-default-styles.html
TEST=http/tests/devtools/elements/styles-2/pseudo-elements.js

Change-Id: I4dd9e8afd448bd5fe237c084d3c0215e91560dd1
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 31, 2020
The CSSWG resolved in w3c/csswg-drafts#4568
that properties like 'text-indent' that don't apply to ::marker, should
not be able to affect the ::marker via inheritance when set to an
ancestor.

Therefore, this patch sets 'text-indent: 0 !important' in UA origin.

Bug: 1031667

TEST=external/wpt/css/css-pseudo/marker-content-023.html
TEST=external/wpt/css/css-pseudo/marker-default-styles.html
TEST=http/tests/devtools/elements/styles-2/pseudo-elements.js

Change-Id: I4dd9e8afd448bd5fe237c084d3c0215e91560dd1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2382750
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/master@{#803126}
@fantasai
Copy link
Collaborator

@Loirooriol Edits checked in, mind reviewing (and closing out the issue if it looks OK)?

@Loirooriol
Copy link
Contributor

Other properties must not have an effect on the [=marker box=]
when set in the [=author origin=] of the [=cascade=].

What about the user origin? I would say no effect either, so that the behavior can be achieved with !important in UA origin.

@MatsPalmgren
Copy link

I'd like to note for the record that I strongly disagree with this resolution.
I'm fine with resetting these properties to their initial value for ::marker in an UA rule, but they should not be !important.
An author should be able to set them and they should have an effect.

@Loirooriol
Copy link
Contributor

@MatsPalmgren But this is just until the outside marker layout model becomes clear. Then it will be fine to allow authors to change the properties. IMO the resolution is a step forwards since it allows several properties that were not allowed beforehand.

@MatsPalmgren
Copy link

This is the wrong way to go about it IMHO. Doing a long series of backward-incompatible changes to how properties work for ::marker is very bad for authors. Also, adding UA rules to ::marker affects both inside and outside markers.
What we should do is to: 1, define the layout model for outside markers, and 2, allow all properties on them, unless there's a very good reason to deny authors to use some of them. This is easy BTW, see #3771.

@faceless2
Copy link
Author

(we should) define the layout model for outside markers,

Very strongly agree with this. ::marker is implemented by Gecko, our engine, PrinceXML, PDFReactor's RealObjects, Antenna House Formatter and Weazyprint - those are the ones I'm aware of. Every one of these has had to guess at the layout rules because they're not properly defined.

I also agree with Mats that it's easy: we do largely what he describes in #3771 (specifically: remove newlines; lay out as if it were an inline-block in infinite available space so content is necessarily on a single line; align inline border box with list-item and block align as if it were on first formatted line of list-item; markers do not interact with floats and may overlap).

However as we all know, the devil is in those details (Gecko trims all trailing whitespace; we don't, Prince doesn't. Prince allows newlines; we don't, Gecko doesn't. Prince adds text-align: end; we don't, not testable in Gecko as width can't be set). Incompatibilities are only going to get worse this longer this is left. I'd hope this issue is a sticking plaster until ::marker layout is fully defined.

@Loirooriol
Copy link
Contributor

@faceless2 BTW, Blink is shipping ::marker in 86 (currently beta). And WebKit shipped it long time ago, but it's a more limited version.
And about text-align, I thought about adding text-align: end !important to prevent inheritance (Gecko sets it to end too, you should be able to test with li{white-space:pre}::marker{content:"a\a b c"}). But it turns out that, in quirks mode, outside markers can occupy the whole line and are not sized with shrink-to-fit. So end was bad in Chromium, I set text-align: start !important instead.

@Loirooriol
Copy link
Contributor

About this resolution:

Other properties which are not explicitly listed as apply to ::marker must not have an effect when set by author.

I'm thinking about the visibility property. It doesn't only apply to text, since it's able to hide box borders and things like that. But forcing visibility: visible !important to prevent inheritance seems like an undesirable breaking change.

Or the cursor property. It depends on the box geometry (basically it can expose the sizes of the marker). But resetting it also seems undesirable, since it has always been inherited. And the marker sizes are also exposed in getComputedStyle anyways.

So I wouldn't prevent their inheritance and it could also make sense to accept them to be set directly on ::marker.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 30, 2020
The CSSWG resolved in w3c/csswg-drafts#4568
that inherited properties that apply to text can be set on ::marker and
should affect the marker text.

Therefore, this patch allows 'line-height'. Note it was already possibly
to set it to the list item and the ::marker would inherit it. This patch
just lets authors set it directly to the ::marker.

Bug: 1031667

TEST=external/wpt/css/css-pseudo/marker-line-height.html
TEST=external/wpt/css/css-pseudo/parsing/marker-supported-properties-in-animation.html
TEST=external/wpt/css/css-pseudo/parsing/marker-supported-properties.html

marker-line-height.html fails in legacy because ::markers with
'content: normal' are not implemented with actual text.

Change-Id: If63095d046150a2b5f76c40fce93fce1c0e7741c
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 27, 2020
The CSSWG resolved in w3c/csswg-drafts#4568
that inherited properties that apply to text can be set on ::marker and
should affect the marker text.
And in w3c/csswg-drafts#5303 it resolved that
'line-height' applies to text (as it already does in Chromium).

Therefore, this patch allows 'line-height' in ::marker. Note it was
already possibly to set it to the list item and the ::marker would
inherit it. Just letting authors set it directly to the ::marker.

Bug: 1031667

TEST=external/wpt/css/css-pseudo/marker-line-height.html
TEST=external/wpt/css/css-pseudo/parsing/marker-supported-properties-in-animation.html
TEST=external/wpt/css/css-pseudo/parsing/marker-supported-properties.html

marker-line-height.html fails in legacy because ::markers with
'content: normal' are not implemented with actual text.

Change-Id: If63095d046150a2b5f76c40fce93fce1c0e7741c
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 27, 2020
The CSSWG resolved in w3c/csswg-drafts#4568
that inherited properties that apply to text can be set on ::marker and
should affect the marker text.
And in w3c/csswg-drafts#5303 it resolved that
'line-height' applies to text (as it already does in Chromium).

Therefore, this patch allows 'line-height' in ::marker. Note it was
already possibly to set it to the list item and the ::marker would
inherit it. Just letting authors set it directly to the ::marker.

Bug: 1031667

TEST=external/wpt/css/css-pseudo/marker-line-height.html
TEST=external/wpt/css/css-pseudo/parsing/marker-supported-properties-in-animation.html
TEST=external/wpt/css/css-pseudo/parsing/marker-supported-properties.html

marker-line-height.html fails in legacy because ::markers with
'content: normal' are not implemented with actual text.

Change-Id: If63095d046150a2b5f76c40fce93fce1c0e7741c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2438413
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821293}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 27, 2020
The CSSWG resolved in w3c/csswg-drafts#4568
that inherited properties that apply to text can be set on ::marker and
should affect the marker text.
And in w3c/csswg-drafts#5303 it resolved that
'line-height' applies to text (as it already does in Chromium).

Therefore, this patch allows 'line-height' in ::marker. Note it was
already possibly to set it to the list item and the ::marker would
inherit it. Just letting authors set it directly to the ::marker.

Bug: 1031667

TEST=external/wpt/css/css-pseudo/marker-line-height.html
TEST=external/wpt/css/css-pseudo/parsing/marker-supported-properties-in-animation.html
TEST=external/wpt/css/css-pseudo/parsing/marker-supported-properties.html

marker-line-height.html fails in legacy because ::markers with
'content: normal' are not implemented with actual text.

Change-Id: If63095d046150a2b5f76c40fce93fce1c0e7741c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2438413
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821293}
pull bot pushed a commit to Alan-love/chromium that referenced this issue Oct 27, 2020
The CSSWG resolved in w3c/csswg-drafts#4568
that inherited properties that apply to text can be set on ::marker and
should affect the marker text.
And in w3c/csswg-drafts#5303 it resolved that
'line-height' applies to text (as it already does in Chromium).

Therefore, this patch allows 'line-height' in ::marker. Note it was
already possibly to set it to the list item and the ::marker would
inherit it. Just letting authors set it directly to the ::marker.

Bug: 1031667

TEST=external/wpt/css/css-pseudo/marker-line-height.html
TEST=external/wpt/css/css-pseudo/parsing/marker-supported-properties-in-animation.html
TEST=external/wpt/css/css-pseudo/parsing/marker-supported-properties.html

marker-line-height.html fails in legacy because ::markers with
'content: normal' are not implemented with actual text.

Change-Id: If63095d046150a2b5f76c40fce93fce1c0e7741c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2438413
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821293}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Nov 4, 2020
…rty in ::marker, a=testonly

Automatic update from web-platform-tests
[css-pseudo] Support 'line-height' property in ::marker

The CSSWG resolved in w3c/csswg-drafts#4568
that inherited properties that apply to text can be set on ::marker and
should affect the marker text.
And in w3c/csswg-drafts#5303 it resolved that
'line-height' applies to text (as it already does in Chromium).

Therefore, this patch allows 'line-height' in ::marker. Note it was
already possibly to set it to the list item and the ::marker would
inherit it. Just letting authors set it directly to the ::marker.

Bug: 1031667

TEST=external/wpt/css/css-pseudo/marker-line-height.html
TEST=external/wpt/css/css-pseudo/parsing/marker-supported-properties-in-animation.html
TEST=external/wpt/css/css-pseudo/parsing/marker-supported-properties.html

marker-line-height.html fails in legacy because ::markers with
'content: normal' are not implemented with actual text.

Change-Id: If63095d046150a2b5f76c40fce93fce1c0e7741c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2438413
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821293}

--

wpt-commits: 91da84149d57c560e3435866fd198d1910c22e3f
wpt-pr: 25886
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Nov 5, 2020
…rty in ::marker, a=testonly

Automatic update from web-platform-tests
[css-pseudo] Support 'line-height' property in ::marker

The CSSWG resolved in w3c/csswg-drafts#4568
that inherited properties that apply to text can be set on ::marker and
should affect the marker text.
And in w3c/csswg-drafts#5303 it resolved that
'line-height' applies to text (as it already does in Chromium).

Therefore, this patch allows 'line-height' in ::marker. Note it was
already possibly to set it to the list item and the ::marker would
inherit it. Just letting authors set it directly to the ::marker.

Bug: 1031667

TEST=external/wpt/css/css-pseudo/marker-line-height.html
TEST=external/wpt/css/css-pseudo/parsing/marker-supported-properties-in-animation.html
TEST=external/wpt/css/css-pseudo/parsing/marker-supported-properties.html

marker-line-height.html fails in legacy because ::markers with
'content: normal' are not implemented with actual text.

Change-Id: If63095d046150a2b5f76c40fce93fce1c0e7741c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2438413
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821293}

--

wpt-commits: 91da84149d57c560e3435866fd198d1910c22e3f
wpt-pr: 25886
@fantasai fantasai added css-lists-3 Current Work and removed css-pseudo-4 Current Work labels Dec 30, 2020
@fantasai
Copy link
Collaborator

Ok, pushed some wording changes:

Let me know if this is workable, or if the wording needs additional tweaking. CC @tabatkins for review.

As for whether this is the right approach or if we should go ahead and define marker layout...
I note @MatsPalmgren’s objection and insistence that we should just define outside marker layout, but as @faceless2 noted “the devil is in those details”. I've worked on a number of layout specs, and I'm familiar with the range of issues that have been raised against outside marker layout over the years and I am quite sure this is not a trivial project. Should we do this? Yes. But in the next level, because the rest of this spec is comparatively done, and we should get it wrapped up and sent to CR.

If anyone wants to start work on the outside marker layout project right now, I invite you to start a css-lists-4 delta ED in the repository and start drafting it up. I can't prioritize it right now personally, but this isn't to say that we shouldn't be working on it. But I think it will take 1-3 years, not 1-3 months, before we shake out all the issues and that's with someone actively working on it. Which is why I don't want to hold up everything else in css-lists-3 (most of which is already shipping) to wait for it.

In the meantime, I think it's best to disable most styling of ::marker until we can work out what exactly it means, so that authors don't build a dependency on particular behaviors that we end up wanting to change as we work through all the outside marker layout issues. If you disagree on this point and would like to argue for letting whatever interop and Web-compat constraints fall out of authors exploiting various layout properties on your implementations dictate what we're able to define for outside marker layout going forward, please make a cogent argument to that effect. But I think it's safest if such properties don't have any effect until we are agreed on what exactly that effect will be.

mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
The CSSWG resolved in w3c/csswg-drafts#4568
that properties like 'text-indent' that don't apply to ::marker, should
not be able to affect the ::marker via inheritance when set to an
ancestor.

Therefore, this patch sets 'text-indent: 0 !important' in UA origin.

Bug: 1031667

TEST=external/wpt/css/css-pseudo/marker-content-023.html
TEST=external/wpt/css/css-pseudo/marker-default-styles.html
TEST=http/tests/devtools/elements/styles-2/pseudo-elements.js

Change-Id: I4dd9e8afd448bd5fe237c084d3c0215e91560dd1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2382750
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/master@{#803126}
GitOrigin-RevId: da2ab72e3cb6d747044f9035d1d8968acd45ca20
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
The CSSWG resolved in w3c/csswg-drafts#4568
that inherited properties that apply to text can be set on ::marker and
should affect the marker text.

Therefore, this patch allows 'hyphens', 'letter-spacing', 'line-break',
'overflow-wrap', 'tab-size', 'word-break', and 'word-spacing' in marker.

Note it was already possibly to set these properties in the list item
and the ::marker would inherit them. This patch just lets authors set
them directly on the ::marker.

I would have allowed 'text-justify' too but it's not enabled by default
and in LayoutNG it has no effect on inline boxes (bug 1124043), so it
didn't seem worth it.

Bug: 1031667

TEST=external/wpt/css/css-pseudo/marker-hyphens.html
TEST=external/wpt/css/css-pseudo/marker-letter-spacing.html
TEST=external/wpt/css/css-pseudo/marker-line-break.html
TEST=external/wpt/css/css-pseudo/marker-overflow-wrap.html
TEST=external/wpt/css/css-pseudo/marker-tab-size.html
TEST=external/wpt/css/css-pseudo/marker-word-break.html
TEST=external/wpt/css/css-pseudo/marker-word-spacing.html
TEST=external/wpt/css/css-pseudo/parsing/marker-supported-properties-in-animation.html
TEST=external/wpt/css/css-pseudo/parsing/marker-supported-properties.html

Some tests fail in legacy because ::markers with 'content: normal' are
not implemented with actual text.

Change-Id: I2086854796fd355928591ee4c85f241337b33445
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2388384
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/master@{#803839}
GitOrigin-RevId: 41cb9e56ce950d5db7947a3108c42ad567817aa3
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
The CSSWG resolved in w3c/csswg-drafts#4568
that properties like 'text-align' that don't apply to ::marker, should
not be able to affect the ::marker via inheritance when set to an
ancestor.

Therefore, this patch sets 'text-align: start !important' in UA origin.
The specific value is not much important, since inside markers are
inline boxes unaffected by 'text-align', and outside markers are usually
sized with shrink-to-fit. It only matters in case of outside markers
whose text contains newline characters, or in quirks mode when the
marker is forced to occupy the whole line.

Additionally, since 'text-align' has been implemented as a longhand
rather than a shorthand of 'text-align-all' and 'text-align-last', this
patch also sets 'text-align-last: start !important'.

Bug: 1031667

TEST=external/wpt/css/css-pseudo/marker-text-align.html
TEST=http/tests/devtools/elements/styles-2/pseudo-elements.js

Change-Id: Ibe8d72d4675c5d1e3b0ceedf3acc615e1514fe7d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2391242
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/master@{#804347}
GitOrigin-RevId: 1d15ab1e2ce7b06f01a8444d7998bdb261a1365b
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
The CSSWG resolved in w3c/csswg-drafts#4568
that inherited properties that apply to text can be set on ::marker and
should affect the marker text.

Therefore, this patch allows text-decoration-skip-ink, text-emphasis,
-webkit-text-emphasis-color, -webkit-text-emphasis-position,
-webkit-text-emphasis-style, and text-shadow in ::marker.

text-underline-position is not allowed despite being inherited because
it doesn't affect underlines specified by ancestor elements. And since
text-decoration-line is not allowed, it would be pointless.

Bug: 1031667

TEST=external/wpt/css/css-pseudo/marker-text-decoration-skip-ink.html
TEST=external/wpt/css/css-pseudo/marker-text-emphasis.html
TEST=external/wpt/css/css-pseudo/marker-text-shadow.html
TEST=external/wpt/css/css-pseudo/parsing/marker-supported-properties-in-animation.html
TEST=external/wpt/css/css-pseudo/parsing/marker-supported-properties.html

Some parsing tests fail because unprefixed text-emphasis hasn't been
implemented. And some reftests fail in legacy because ::markers with
'content: normal' are not implemented using actual text.

Change-Id: I3bd7287f0e8164a7fac0ed84dd4ec0af57a34f2e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2396125
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805343}
GitOrigin-RevId: 896f37a4d7d864e241b36fefda634f509b2d5911
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
The CSSWG resolved in w3c/csswg-drafts#4568
that inherited properties that apply to text can be set on ::marker and
should affect the marker text.
And in w3c/csswg-drafts#5303 it resolved that
'line-height' applies to text (as it already does in Chromium).

Therefore, this patch allows 'line-height' in ::marker. Note it was
already possibly to set it to the list item and the ::marker would
inherit it. Just letting authors set it directly to the ::marker.

Bug: 1031667

TEST=external/wpt/css/css-pseudo/marker-line-height.html
TEST=external/wpt/css/css-pseudo/parsing/marker-supported-properties-in-animation.html
TEST=external/wpt/css/css-pseudo/parsing/marker-supported-properties.html

marker-line-height.html fails in legacy because ::markers with
'content: normal' are not implemented with actual text.

Change-Id: If63095d046150a2b5f76c40fce93fce1c0e7741c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2438413
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821293}
GitOrigin-RevId: cd65c12e6f6656135a93a57b339a4cffeb3684c1
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
@frivoal @fantasai @faceless2 @astearns @MatsPalmgren @Loirooriol @css-meeting-bot and others