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-ruby] replaced annotation containers and base containers are nonsensical #6000

Closed
frivoal opened this issue Feb 16, 2021 · 5 comments
Closed

Comments

@frivoal
Copy link
Collaborator

frivoal commented Feb 16, 2021

It doesn't seem to make sense to allow a replaced element to be a base container or an anotation container. Suspect the easiest thing to do is to adjust the box fixup so that replaced base containers get turned into replaced bases with an anonymous base container around them (and the analogous thing for annotations). Should we just do that, or something else?

Also, how should this affect / not affect is computed display value?

@frivoal frivoal added the css-ruby-1 Current Work label Feb 16, 2021
@frivoal frivoal added the Agenda+ label Apr 6, 2021
@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-ruby] replaced annotation containers and base containers are nonsensical, and agreed to the following:

  • RESOLVED: if you are a replaced element with dipslay of base or annotation container without changing the computed value they're treated as base or annotation respectively
The full IRC log of that discussion <dael> Topic: [css-ruby] replaced annotation containers and base containers are nonsensical
<dael> github: https://github.com//issues/6000
<dael> florian: Ruby, when we're rendering it's hierarchy of boxes. Externally ruby contain box. Then base containers and annotation containers and in those bases and annotations
<dael> florian: What happens if you use display to set a replaced element to be a replaced container. They're replaced so won't contain anything
<dael> florian: Spec does not take that into account. Doesn't say anything useful
<dael> florian: I think we should do a correction. Computed or used time. If you have replaced element set to base or annotation container they're turend into base or annotation respectively
<dael> florian: There's a series of box fixup which makes sure if item is unparent the right hierarcy of anon boxes is generated. That correction is suffient and the rest of the steps would make sure it has the right htings around it
<dael> florian: IMportant because base and annotation containers are only barely boxes. Fairly abstract. They're derived from element but properties is limited. More of a rectangle then abox. Trying ot render replaced element as those is not defined.
<dael> florian: Innermost element is a box in a tree, but intermediary is more a helper then a box you can style.
<dael> astearns: CommentS? Concerns?
<dael> astearns: I assume this prop should have test case? Otherwise I expect it would be lost in impl
<dael> florian: Sure. Doesn't have but happyt o add
<dael> emilio: Do you know how this behaves on FF?
<dael> emilio: We don't do anything special, just create a replaced box. Display doesn't effect box you created. Does this need special case?
<dael> fantasai: You treat as inline replaced box? Outer display can be inline or block or flex...lot of roles it can play. Question here it doesn't seem to make a whole lot of sence for it to play role of ruby annotation container. Could define it to work but then have to define.
<dael> emilio: I think prob treat as an inline outside thing. Worth checking. I misunderstood issue. Don't mind saying treated as whatever. Should not effect display-type. What happens if you style inline with table cell.
<dael> florian: If you do at used I don't think issue
<dael> emilio: I think we do at used value for every other. If you put img and say display table caption I think computed is table-caption but we treat as inline or block. Just need to decide
<dael> florian: Doing it at used value time seems fine. I propose we do that. If you set annotation container it's an annotation at used time. Everything else as it exists. Same for base
<dael> emilio: Sounds fine
<dael> astearns: Is prop that at used value time we get the appropriate anon box for base and annotation?
<dael> florian: don't need that part. there is box fixup that lets us generate container. We need to say if you are a replaced element with dipslay of base or annotation container without changing the computed value they're treated as base or annotation respectively
<dael> astearns: Is that & good for resolution florian ?
<dael> s/&/^
<dael> Prop: if you are a replaced element with dipslay of base or annotation container without changing the computed value they're treated as base or annotation respectively
<dael> astearns: Any additional comments or concerns?
<dael> dholbert: Questions. First, we should make sure the box fixup code reacts to used value and not computed
<dael> florian: Sure
<dael> dholbert: I wonder if display ruby for outermost container do we need something like that?
<dael> florian: I remember thinking about it and concluding no problem in that case, but don't remember why
<dael> astearns: If it ends up being an issue we can add it
<dael> florian: I'll think about it. I'm not remembering why I concluded it wasn't
<dael> astearns: dholbert are you okay resolving jsut for the two values?
<dael> dholbert: Yeah, fine. Makes sense to handle separately
<dael> astearns: Other comments?
<dael> astearns: Prop: if you are a replaced element with dipslay of base or annotation container without changing the computed value they're treated as base or annotation respectively
<dael> astearns: Obj?
<dael> RESOLVED: if you are a replaced element with dipslay of base or annotation container without changing the computed value they're treated as base or annotation respectively
@upsuper
Copy link
Member

upsuper commented Apr 24, 2021

I don't think I agree with this decision.

It doesn't sound useful to define the behavior this way. Why would any reasonable developer would want to assign a ruby internal display value to a replaced element? If they want to see such element in ruby base or annotation, just drop it into a <rb> or <rt>. Using those special value with replaced element also easily discards the semantics of ruby for non-CSS consumers.

In addition, it doesn't seem to me that we have any precedence for this, e.g. we don't handle replaced element with table internal display values specially, fixing up them so that they can form part of the table, do we? I don't quite see why it's important to handle ruby internal display values specially. This would likely require impls to add extra code for an edge case which may just never happen.

But I don't know how replaced elements should be handled in general with weird display values. If it's not defined anywhere, it's probably better to be defined in a general term, rather than case-by-case for each display values. I suggest that we just ignore the display internal values, and treat them as inline like what Gecko does currently.

@frivoal frivoal added css-tables-3 Current Work css-display-3 Current Work Agenda+ labels Apr 26, 2021
@frivoal
Copy link
Collaborator Author

frivoal commented Apr 26, 2021

While I think the spec needs to handle this situation in some way, I am not strongly attached to the particular way we just resolved on, and your suggestion seems good: saying that such elements should be treated as display inline, and letting the box fix-up give them appropriate parents. That would be inline with what the table spec does since #508.

I suppose that we could solve this generically by having this requirement in in css-display in the https://drafts.csswg.org/css-display-3/#layout-specific-display section (loosely inspired by the text at the end of https://drafts.csswg.org/css-tables/#table-structure):

When the 'display' property of a [=replaced element=] computes to one of the [=layout-internal=] values, it is handled as having a used value of ''display/inline''. White space collapsing and anonymous box generation must happen around those replaced elements based on that ''display/inline'' value, as if they never had a [=layout-internal=] display value applied to them.
Advisement: Authors should not assign a [=layout-internal=] display value to [=replaced elements=].

Then, css-ruby (and css-tables) could just refer to it.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-ruby] replaced annotation containers and base containers are nonsensical, and agreed to the following:

  • RESOLVED: Undo previous resolution. Treat replaced elements with an internal ruby display value as if they were inline at used value time
The full IRC log of that discussion <dael> Topic: [css-ruby] replaced annotation containers and base containers are nonsensical
<dael> github: https://github.com//issues/6000
<dael> Rossen_: Let's timebox to 5m
<dael> florian: We discussed that it didn't make a whole lot of sense for replace elements to have ruby-base or ruby-text. xidorn has pushed back
<dael> florian: Would rather we treat as inline replaced elements. Invokes rest of box fixup. Anon parents still generated. This would be simplier and is what happens with tables already. Replaced elements in table only get to be inline and wrapped into table parts
<dael> florian: I think emilio also argued for that.
<dael> florian: Givent he pushback and we do this for tables I think this is a good idea. Saying they're treated as inlines makes sense
<dael> florian: If we do that I would also propose we move this into the display spec instead of being in tables and ruby. Display talks about layout internals and could say when you try and apply to replaced element it's treated as inline
<dael> florian: Any problems with that?
<dael> Rossen_: Did anyone follow it?
<dael> Rossen_: florian since there's no issues or comments, prop?
<dael> florian: Prop: Undo previous resolution. Treat replaced elements with an internal ruby display value as if they were inline
<dael> florian: at used value time
<dael> Rossen_: Would this align closer with table?
<dael> florian: Yes, closer with tables on Moz current impl
<dael> Rossen_: If I have display:table on the replaced element it becomes inline?
<dael> florian: display: table-row or -cell they're inline. For outermost like display:table or ruby this is handled.
<dael> florian: For internal you're treated as inline
<dael> Rossen_: Right. Awesome
<dael> Rossen_: Additional comments or objections?
<dael> RESOLVED: Undo previous resolution. Treat replaced elements with an internal ruby display value as if they were inline at used value time
<dael> Rossen_: Did previous one make it into spec?
<dael> florian: Not yet
@frivoal frivoal self-assigned this Apr 28, 2021
frivoal added a commit that referenced this issue Apr 29, 2021
 Layout-internal replaced elements behave as inline, while the outer
 ruby or table elements behave according to their outer display type.

 This addresses #6000 and refactors the edits made to implement #508
 from css-table to css-display, so that they can be shared between
 css-ruby and css-tables.
@frivoal
Copy link
Collaborator Author

frivoal commented Apr 29, 2021

Solved by 2297b7b.

Will close this issue soon unless someone thinks I got it wrong.

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