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-logical-1] Order of inheritance vs. mapping in logical properties #3029

Closed
fantasai opened this issue Aug 17, 2018 · 17 comments
Closed
Labels
Closed Accepted by CSSWG Resolution css-logical-1 Current Work i18n-tracker Group bringing to attention of Internationalization, or tracked by i18n but not needing response.

Comments

@fantasai
Copy link
Collaborator

Example from @FremyCompany:

  <div style="margin-left: 10px; margin-inline-start: 20px; direction: rtl">
    <div style="margin-right: inherit; margin-inline-start: inherit; direction: ltr"></div>
  </div>

It's clear that margin-right inherits 20px from the computed margin-right value on the parent. What about margin-inline-start, though? Does it inherit 10px or 20px?

@fantasai fantasai added the css-logical-1 Current Work label Aug 17, 2018
@dbaron
Copy link
Member

dbaron commented Aug 17, 2018

It might even be clearer with the following two slightly simpler examples:

<div style="margin-left: 10px; margin-right: 20px; direction: rtl">
  <div style="margin-left: inherit /* compat requires 10px */; direction: ltr"></div>
</div>
<div style="margin-left: 10px; margin-right: 20px; direction: rtl">
  <div style="margin-inline-start: inherit /* 10px or 20px? */; direction: ltr"></div>
</div>

In other words, does inherit inherit the computed physical value from the parent (always), or does it inherit from whichever of physical or logical was specified?

I would hope, at least, that those two examples produce the same results (respectively) as these two:

<div style="margin-inline-end: 10px; margin-inline-start: 20px; direction: rtl">
  <div style="margin-left: inherit; direction: ltr"></div>
</div>
<div style="margin-inline-end: 10px; margin-inline-start: 20px; direction: rtl">
  <div style="margin-inline-start: inherit; direction: ltr"></div>
</div>

since the outer div always has the same computed values for all properties (the difference in that in these last two, it got those values through logical properties rather than physical ones).

@fantasai
Copy link
Collaborator Author

Just wanted to note here that the possible solutions to #3030 depend on what happens here.

@fantasai
Copy link
Collaborator Author

I would hope, at least, that those two examples produce the same results (respectively) as these two.

Yes, they would have to: inheritance uses the parent’s computed value, and pairs of corresponding logical/physical properties are required to have the same computed value.

@Loirooriol
Copy link
Contributor

What about margin-inline-start, though? Does it inherit 10px or 20px?

Chromium, WebKit and Firefox are interoperable, it inherits 10px. So if margin-inline-start resolves to margin-left, it inherits the margin-left of the parent, even if that's the margin-inline-end of the parent.

@fantasai fantasai added the i18n-tracker Group bringing to attention of Internationalization, or tracked by i18n but not needing response. label Sep 15, 2020
@fantasai
Copy link
Collaborator Author

@Loirooriol Thanks! We should probably check with i18n that it's acceptable behavior, but as long as it is, speccing interop sounds reasonable to me.

@r12a
Copy link
Contributor

r12a commented Sep 15, 2020

fwiw, last Friday i had created about 100 tests for css-logical, including the various directional permutations. It's geek week now, so i may upload the tests with results this week, or it may have to wait until next week. I might be able to add a test for this scenario, too.

@r12a
Copy link
Contributor

r12a commented Sep 30, 2020

So the main body of tests and results is at https://w3c.github.io/i18n-tests/results/css-logical

I didn't yet integrate the tests for inheritance into that page, because i'm not sure about the result. You can find 2 tests at

The results are indeed interoperable across Firefox, Chrome, and Safari browsers.

However, i wrote the assertions following the logic that seemed sensible to me: that is that the margin-inline-start for a LTR block (ie. on the left) would inherit the value for the margin on the right (ie. at the start) of a parent RTL block. Under that criterion, all the tests fail.

It seems somewhat illogical, as it were, that what is being inherited by a logical value is not the start (ie. logical) value of the parent. It would mean that a content author has to do some mental gymnastics to decide whether or not to use inherit explicitly.

@astearns
Copy link
Member

@r12a would you be available for any of our remaining meetings this week to discuss this issue?

https://wiki.csswg.org/planning/tpac-2020#schedule

@fantasai
Copy link
Collaborator Author

fantasai commented Oct 20, 2020

So these are the two operations under considerations:

  • Blue Line: Child’s margin-inline-start maps to margin-right which then inherits from parent’s margin-right (which is the parent’s margin-inline-end). This is what browsers currently do.
  • Red Line: Child’s margin-inline-start (which is the child’s margin-right) inherits from parent’s margin-inline-start (which is matched with parent’s margin-left). This is what @r12a expects.

Keep in mind that properties are first cascaded (all together, logical and physical mixed and mapped by writing mode, latest declaration of a conflicting set winning), and then computed (which causes each matching pair to share a single computed value).

logical-inheritance

@Loirooriol
Copy link
Contributor

Note that if logical properties inherit from logical properties, we need to define what happens with all: inherit, since the order will matter. For backwards compatibility it should either not include logical properties, or it include them before physical ones.

I think it would be simpler to just keep what's currently implemented (inherit from the resolved physical properties).

@FremyCompany
Copy link
Contributor

@Loirooriol We resolved yesterday that shorthand properties only apply to one type of physical-or-logical mapping.

During parsing of a css declaration block, shorthands (like margin and all) expands only to the set of longhand (grouped by mapping logic: so either logical or physical) required to describe their value, and by default resort to use physical properties if both sets are possible. Note that they can only expand if they do not contain var() substitutions.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed css logical properties ordering, and agreed to the following:

  • RESOLVED: inheritance of logical properties inherts the corresponding logical proeprty on the parent
The full IRC log of that discussion <TabAtkins> Topic: css logical properties ordering
<astearns> github: https://github.com//issues/3029
<TabAtkins> fantasai: Probably best to look at this diagram at the bottom
<fantasai> https://github.com//issues/3029#issuecomment-712953765
<TabAtkins> fantasai: We've got a set of logical props and physical props
<TabAtkins> fantasai: So if we ltr parent and rtl child
<TabAtkins> fantasai: ltr parent has some set of properties on it that give it margins on either side
<TabAtkins> fantasai: and the child has specified "margin-start: inhert"
<TabAtkins> fantasai: So what does that mean
<TabAtkins> fantasai: From the parent's left margin (the parent's start) or the right amrgin (the child's start)
<myles> ```
<TabAtkins> fantasai: Currently impls map start/end to a physical side on the child, using the child's writing mode, then inherit from the parent's corresponding physical margin
<TabAtkins> fantasai: The other possible behavior, which is possibly better, is that the child inherits its start value from the start value of the parent
<myles> <div id="outer" style="padding-left: 20px padding-right: 30px;" dir="ltr">
<myles> <div id="inner" style="padding-inline-end: inherit" dir="rtl"></div>
<myles> </div>
<TabAtkins> fantasai: Note taht by the time we ahve computed values, which is what we inherit, the left&start margins are set to the same value, and the right&end are set to the same, regardless of how they were specified
<TabAtkins> florian: We don't have any property that is logical & inherited where this shows up without you explicitly saying 'inherit', right?
<TabAtkins> fantasai: Right, not so far.
<myles> q?
<TabAtkins> fantasai: But if we did, I suspect we'd want the start margin of the child to inherit from the start margin of the parent, not from the amtching physical side
<myles> q+
<TabAtkins> fantasai: That's my suspicion, at least
<astearns> zakim, open queue
<Zakim> ok, astearns, the speaker queue is open
<TabAtkins> fremy: That's also my assessment
<myles> q+
<TabAtkins> fremy: If you wanted to inherit the right margin, you'd have said "margin-right". If you say margin-start, you want the start margin
<TabAtkins> fremy: If you had text-indent, you want the text-indent value, whether it's ltr or rtl.
<astearns> ack myles
<TabAtkins> myles: We're positive on this change to have the writing mode resolved based on the parent, not the child, for inheritance
<TabAtkins> myles: Two reasons for this
<TabAtkins> myles: fremy gave an argumen tfrom the author's perspective
<TabAtkins> myles: But i think the user wants it too - if they're reading a book where paragraphs have a padding on one side, and one paragraph is on a different side, the padding should probably flip sides
<TabAtkins> s/on a different side/has a different wm/
<TabAtkins> myles: Second is that it makes logical props more of a first-class citizen
<TabAtkins> myles: This means inheritance doesn't "prefer" physical props, you inherit what's specified
<TabAtkins> myles: That's good for the world and authors, but also good for impls
<TabAtkins> myles: Right now our code is a little more complicated than it could be to make it prefer physical props
<fantasai> s/specified/specified, so that logical properties are more of a first-class citizens/
<TabAtkins> astearns: I'm assuming, r12a, that you're okay with this proposal?
<fantasai> /citizens/citizen/
<TabAtkins> r12a: Yes, I think I suggested this in the first place, so I'm happy with it
<emilio> q+
<astearns> ack emilio
<TabAtkins> proposed resolution: inheritance of logical properties inherts the corresponding logical proeprty on the parent
<TabAtkins> emilio: A bit skeptical that this is easier to implement. you only store one value in the computed style, a physical one
<TabAtkins> emilio: So this breaks some computed-style optimizations
<myles> qq+
<TabAtkins> emilio: When two props map to one value, you have to choose one or the other, and right now impls agree with using the physical one, which they store in the style
<TabAtkins> emilio: So this means you have to add a pass after applying declarations, to know if your writing mode is different, you should inherit them differently
<TabAtkins> astearns: So you're saying the impl may be more difficult than stated, but are you objecting?
<astearns> ack myles
<Zakim> myles, you wanted to react to emilio
<TabAtkins> emilio: I think it woudl be unfortunate, especially given current interop, but I dont' think I have an objection until I try to implement and it's gross
<TabAtkins> myles: So you mentioned that we only store one set of values. That's true; netiehr proposal requires us to store multiple values
<TabAtkins> emilio: Right, but when you inherit a style you have a coypable part of the style which is what inherits
<TabAtkins> emilio: All impls separate inherited data and non-inherited
<TabAtkins> emilio: So if any of them come from a logical prop, you'll need to do the work and inherit it specially with the writing mode of the parent
<TabAtkins> emilio: So it breaks the simple "pointer bump" impl
<TabAtkins> myles: I agree that would be an issue if this was a common thing
<TabAtkins> emilio: Right. So I think maybe your'e just moving a special case from on eplace to another, but...
<TabAtkins> astearns: So, any objections?
<myles> q?
<TabAtkins> RESOLVED: inheritance of logical properties inherts the corresponding logical proeprty on the parent
<TabAtkins> <br until=":30">

@Loirooriol
Copy link
Contributor

[=Shorthand properties=] that encompass both logical and physical longhands
(such as the 'all' shorthand)
set omitted values first,
and otherwise set the physical values last.

@fantasai I don't really get this. Which "omitted values"? "otherwise" which condition?

@fantasai
Copy link
Collaborator Author

@Loirooriol If there's a shorthand that can set both physical and logical values, e.g. suppose we extended margin in such a fashion, then whichever longhands are omitted (not set explicitly) should be set first so that the explicit values reset whatever is necessary. So if someone specifies margin: relative 1em then we set the logical longhands last so they can win against the physical ones. If we always set physical last, then setting logical values in such a shorthand would be impossible.

@Loirooriol
Copy link
Contributor

@fantasai But according to #3030,

During parsing of a css declaration block, shorthands (like margin and all) expands only to the set of longhand (grouped by mapping logic: so either logical or physical)

while you seem to mean that both sets of longhand are set? What is supposed to happen if I do

document.body.style.cssText = "margin: 0";
document.body.style.length; // 4 or 8 ??

@FremyCompany
Copy link
Contributor

My understanding is 4, the logical properties only. But this is understable since such property doesn't exist yet...

@Loirooriol
Copy link
Contributor

Well, margin already exists. When not including the non-stable logical keyword, should it only expand to the physical properties only, or should it also reset the logical ones beforehand? From @fantasai's explanation the spec seems to want the latter, but I'm not sure if this is web compatible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closed Accepted by CSSWG Resolution css-logical-1 Current Work i18n-tracker Group bringing to attention of Internationalization, or tracked by i18n but not needing response.
Projects
None yet
Development

No branches or pull requests

7 participants