Nothing Special   »   [go: up one dir, main page]

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-align] Special case for inline-block+scroll-container elements needs to cover inline blocks that **contain** scroll containers #3611

Closed
dholbert opened this issue Feb 4, 2019 · 13 comments
Labels
Closed Accepted by Editor Discretion Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. CSS2 css-align-3 Current Work

Comments

@dholbert
Copy link
Member
dholbert commented Feb 4, 2019

This is about the special case described at the end of the "block" section, here:
https://drafts.csswg.org/css-align/#baseline-export
which was added in #2902.
Right now, that spec text says:

...for legacy reasons, if an inline-block is a scroll container or contains no in-flow line boxes, its first and last baseline sets are synthesized from its margin box.

It appears we need to broaden the first part of that condition.

Right now, the special case specifically covers scenarios where the inline-block is a scroll container. But it doesn't cover the case where the inline-block contains a scroll container (and nothing else). Browsers render that scenario the same way, but the spec doesn't say they should, AFAICT.

i.e. Browsers render these two divs the same (using their margin-bottom edge for baseline alignment):

abc
<div style="display:inline-block;overflow:scroll">...</div>
<div style="display:inline-block"><div style="overflow:scroll">...</div></div>

Here's a real-world testcase demonstrating that browsers do in fact align them the same (using the margin-bottom for both): https://hg.mozilla.org/mozilla-central/raw-file/2064e7c799d2/layout/reftests/inline/inline-block-baseline.html

CC @tabatkins @fantasai

@dholbert dholbert added the css-align-3 Current Work label Feb 4, 2019
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Feb 6, 2019
…crolled content (unless their display value is block-inside). r=mats

Before this patch, we made scrollable frames derive their baseline from their
margin-box, because that's what the spec requires for scrollable inline-block
boxes. However, the spec now singles out inline-block as a special case, and
other sorts of scrollable inline-level containers are supposed to derive their
baseline from the scrolled content. So, this patch makes us do that, with an
exception for scrollable inline-block boxes.

For more info about the block-inside special case, see the end of the "block
containers" chunk here: https://drafts.csswg.org/css-align/#baseline-export
(Though that spec text may be a bit too specific, per my spec issue at
w3c/csswg-drafts#3611 -- that's why this patch checks
for block-inside rather than inline-block.)

Differential Revision: https://phabricator.services.mozilla.com/D18481

--HG--
extra : moz-landing-system : lando
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 7, 2019
…ent (unless their display value is block-inside).

Before this patch, we made scrollable frames derive their baseline from their
margin-box, because that's what the spec requires for scrollable inline-block
boxes. However, the spec now singles out inline-block as a special case, and
other sorts of scrollable inline-level containers are supposed to derive their
baseline from the scrolled content. So, this patch makes us do that, with an
exception for scrollable inline-block boxes.

For more info about the block-inside special case, see the end of the "block
containers" chunk here: https://drafts.csswg.org/css-align/#baseline-export
(Though that spec text may be a bit too specific, per my spec issue at
w3c/csswg-drafts#3611 -- that's why this patch checks
for block-inside rather than inline-block.)

Differential Revision: https://phabricator.services.mozilla.com/D18481

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=969874
gecko-commit: 21b74260c8a9fcc8c35ffafc2f98254753fe8d7a
gecko-integration-branch: central
gecko-reviewers: mats
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 7, 2019
…ent (unless their display value is block-inside).

Before this patch, we made scrollable frames derive their baseline from their
margin-box, because that's what the spec requires for scrollable inline-block
boxes. However, the spec now singles out inline-block as a special case, and
other sorts of scrollable inline-level containers are supposed to derive their
baseline from the scrolled content. So, this patch makes us do that, with an
exception for scrollable inline-block boxes.

For more info about the block-inside special case, see the end of the "block
containers" chunk here: https://drafts.csswg.org/css-align/#baseline-export
(Though that spec text may be a bit too specific, per my spec issue at
w3c/csswg-drafts#3611 -- that's why this patch checks
for block-inside rather than inline-block.)

Differential Revision: https://phabricator.services.mozilla.com/D18481

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=969874
gecko-commit: 21b74260c8a9fcc8c35ffafc2f98254753fe8d7a
gecko-integration-branch: central
gecko-reviewers: mats
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 3, 2019
…crolled content (unless their display value is block-inside). r=mats

Before this patch, we made scrollable frames derive their baseline from their
margin-box, because that's what the spec requires for scrollable inline-block
boxes. However, the spec now singles out inline-block as a special case, and
other sorts of scrollable inline-level containers are supposed to derive their
baseline from the scrolled content. So, this patch makes us do that, with an
exception for scrollable inline-block boxes.

For more info about the block-inside special case, see the end of the "block
containers" chunk here: https://drafts.csswg.org/css-align/#baseline-export
(Though that spec text may be a bit too specific, per my spec issue at
w3c/csswg-drafts#3611 -- that's why this patch checks
for block-inside rather than inline-block.)

Differential Revision: https://phabricator.services.mozilla.com/D18481

UltraBlame original commit: 21b74260c8a9fcc8c35ffafc2f98254753fe8d7a
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 3, 2019
…crolled content (unless their display value is block-inside). r=mats

Before this patch, we made scrollable frames derive their baseline from their
margin-box, because that's what the spec requires for scrollable inline-block
boxes. However, the spec now singles out inline-block as a special case, and
other sorts of scrollable inline-level containers are supposed to derive their
baseline from the scrolled content. So, this patch makes us do that, with an
exception for scrollable inline-block boxes.

For more info about the block-inside special case, see the end of the "block
containers" chunk here: https://drafts.csswg.org/css-align/#baseline-export
(Though that spec text may be a bit too specific, per my spec issue at
w3c/csswg-drafts#3611 -- that's why this patch checks
for block-inside rather than inline-block.)

Differential Revision: https://phabricator.services.mozilla.com/D18481

UltraBlame original commit: 21b74260c8a9fcc8c35ffafc2f98254753fe8d7a
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 4, 2019
…crolled content (unless their display value is block-inside). r=mats

Before this patch, we made scrollable frames derive their baseline from their
margin-box, because that's what the spec requires for scrollable inline-block
boxes. However, the spec now singles out inline-block as a special case, and
other sorts of scrollable inline-level containers are supposed to derive their
baseline from the scrolled content. So, this patch makes us do that, with an
exception for scrollable inline-block boxes.

For more info about the block-inside special case, see the end of the "block
containers" chunk here: https://drafts.csswg.org/css-align/#baseline-export
(Though that spec text may be a bit too specific, per my spec issue at
w3c/csswg-drafts#3611 -- that's why this patch checks
for block-inside rather than inline-block.)

Differential Revision: https://phabricator.services.mozilla.com/D18481

UltraBlame original commit: 21b74260c8a9fcc8c35ffafc2f98254753fe8d7a
@tabatkins
Copy link
Member

You're right that the first condition isn't sufficient, but some testing by me and @fantasai seems to show that it should be broadened in a different way.

If the inline-block contains a scrollable flexbox, it still gets baseline-aligned per its content:

<!DOCTYPE html>
<style>
.inline-block { display: inline-block; }
.test {
  background-color: lightgrey;
  overflow: hidden;
}
.border { border: solid blue;  border-width: 1px 3px 5px 7px; }
.padding { padding: 10px; }
.margin { margin: 1px 3px 5px 7px; }
</style>

The boxes should align vertically:
<div class="inline-block">
  <div>
      <div class="test border padding margin" style="display:flex">X</div>
  </div>
</div>

<div class="test inline-block margin border padding">X</div>

So it looks like the condition, rather than "inline-blocks that are scroll containers", should be "block containers that are scroll containers".

That covers the original condition, and makes your testcase valid per spec, as the inline-block tries to determine its baselines from its content, sees only a scrollable block container, and concludes that it has no baselines. (And then, per Inline, it synthesizes baselines from its own margin box.)

Does this sound correct?


Aside: Form controls are fun here, as semi-replaced elements. input is baseline-aligned (and can't be made scrollable), but textarea and <select multiple> margin-align (because they're inherently scrollable). This should be written down somewhere...

@dholbert
Copy link
Member Author

I think that sounds correct, yeah. Thanks!

@bfgeek
Copy link
bfgeek commented Jan 22, 2020

input is slightly more fun, in that even though it might not have any content, it'll still "baseline" align, e.g. https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=7691

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed Special case for inline-block+scroll-container elements needs to cover inline blocks that **contain** scroll containers.

The full IRC log of that discussion <heycam> Topic: Special case for inline-block+scroll-container elements needs to cover inline blocks that **contain** scroll containers
<heycam> github: https://github.com//issues/3611
<heycam> TabAtkins: not sure if this needs discussion, dholbert agreed
<heycam> ScribeNick: heycam
<heycam> TabAtkins: questions from dholbert was about baselines for inline-blocks in certain cases
<heycam> ... the baseline export section, block containers, it says for legacy reasons [ ... ] the baseline is synthesized from the margin box
<heycam> ... dholbert found this doesn't cover what browsers do today
<heycam> ... there are some cases where it does look inside to elements for a baseline
<heycam> ... we found a fix
<heycam> ... rather than testing for inline block being a scroll container, change it to if it has a child that is a scroll container
<heycam> iank_: should be any descendant
<heycam> astearns: that's not what's in the issue though
<fantasai> here's the change https://github.com/w3c/csswg-drafts/commit/91af157d21c3323d031ce2537d8dfba632b099ca
<heycam> TabAtkins: if the ineline block is a scroll container, it works properly
<fantasai> sorry, wrong changeset
<heycam> ... but dholbert found if it contains a scroll container and nothing else, the only child, you still shouldn't look at that child for baseline info, just synthesize
<heycam> ... we discovered that the condition is different -- some scrollable blocks still do influence the baseline
<iank_> https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=7695
<heycam> fantasai: it should be the first in-flow item [...]
<heycam> dbaron: only if the scroll container is block level
<heycam> TabAtkins: scrollable flex boxes that are child of an inline block still give their baseline info
<heycam> ... it's just block containers that hide themselves
<heycam> ... inline blocks and blocks
<heycam> emilio: in Gecko, the baseline of a scrolling box is synthesized from the margin box if the scrolled thing is block level, or you have contain layout
<heycam> ... so I think there may be an easier way to define this
<heycam> ... the scroll box synthesizing the baseline from the margin box, not by looking inside
<heycam> dbaron: is your proposal the thing in the "13 days ago" comment
<heycam> TabAtkins: yes
<heycam> dbaron: my understanding is that this fixes it because it's fixing the recursive invocation of the same alg
<heycam> ... think this makes sense
<heycam> iank_: still applies for any arbitrary child?
<heycam> iank_: I think this special scroll behavior is only executed in the inline block behavior
<heycam> ... looking at the last baseline
<heycam> TabAtkins: if anyone has any possible complications, please comment on the issue
<fantasai> https://drafts.csswg.org/css-align-3/#baseline-export
<heycam> fantasai: I think the change we're planning to make would be in this section
<fantasai> "However, for legacy reasons, if an inline-block is a scroll container or contains no in-flow line boxes, its first and last baseline sets are synthesized from its margin box."
<heycam> fantasai: we'd say it has no baseline set
<heycam> ... and that would trigger the appropriate behavior
<heycam> iank_: if you have multiple children
<heycam> ... two children, one scrollable container, then some text, you'd pick up the baseline from the text?
<heycam> ... if it didn't have a baseline set?
<heycam> fantasai: we're requesting the baseline of a box. if it says it doesn't have one, it'll synthesize
<heycam> iank_: there's a subtle difference between synthesizing a baseline for a box vs something like providing a baseline
<heycam> ... some things like tables inside of an atomic inline context, they don't synthesize a baseline and they get skipped for this calculation
<heycam> ... but scrollable containers don't get skipped
<heycam> fantasai: each formatting context says "this is how I find my baseline to export it out", and table will say my exported baseline is the first row or whatever
<heycam> ... but for block containers, it'll look at the first baseline, skipping tables etc., the next element that's there and ask it
<heycam> iank_: the scrollable container the baseline is exports out is the block-end offset
<heycam> iank_: when we enter into an atomic inline block, we set a bit on input saying "I need the last baseline since I'm in an atomic inline block"
<heycam> ... and that triggers this special behavior
<heycam> ... it propagates through block level block containers
<heycam> fantasai: going forward we'll have an option to choose first or last baseline
<heycam> iank_: that's fine
<heycam> fantasai: I'll make a PR with some proposed text and we can go over it later
<heycam> astearns: since this is a spec change should help match reality, can it come with tests?
<heycam> TabAtkins: yes

@astearns astearns removed the Agenda+ label Jan 23, 2020
@fantasai
Copy link
Collaborator
fantasai commented Mar 6, 2020

@jonjohnjohnson
Copy link
jonjohnjohnson commented Mar 12, 2020

If scrollable overflow only happens in either the inline or block axis, would that afford baseline alignment in different ways?

@fantasai
Copy link
Collaborator

@fantasai
Copy link
Collaborator

@fantasai
Copy link
Collaborator

OK, we've committed some changes to fix the spec here. @bfgeek @dholbert Would you mind reviewing?

@tabatkins
Copy link
Member
tabatkins commented Apr 28, 2020

Okay, edits were made in 7a66d53 (paired with 3bd261f to clarify exactly when baselines are synthesized). After puzzling over Elika's testcase a bit (now captured in the repo at https://drafts.csswg.org/css-align-3/baseline-align-test), we realized the actual behavior here is weird but interoperable:

  • block-containers that are scrollable have a last baseline set, all of which are set to be the bottom margin edge of the box. (This is different from normal synthesis rules; in particular, the central baseline is also the bottom margin edge, rather than halfway between the top and bottom margin edge.)

    You can see the difference from your suggested "block container containing only a scroll container" by looking at cases B (inline-block scroll container) and I (inline-block containing only a scroll container) in Elika's testcase - these correspond to the two boxes in your testcase from the first comment. Your testcase didn't give the wrapper inline-block any dimensions, so it couldn't tell the difference between the two cases like Elika's can - if everything has borders, you can clearly see that it does baseline-align to the child scroller, not the inline-block wrapper.

  • block-containers that are scrollable do not have a first baseline set. Again look at cases B and I, but for the table/flex rows - in both cases, the outermost element is synthesizing a baseline from its box.

@dholbert
Copy link
Member Author

Thanks for that explanation! I think your bullet-points there make sense and seem to mesh with my observations from fantasai's testcase.

RE the spec edits: they make sense if I don't think about writing-modes at all. :)

But if I take writing-mode into consideration, then 7a66d53 doesn't quite make sense -- in a vertical writing-mode, I don't think it's right (or meaningful) to say that we derive the last baseline set from the margin-bottom edge... Perhaps that means to say that we derive it from the "block-end margin edge"?

That seems to be what Chrome uses for cases "B" and "I" if I load https://drafts.csswg.org/css-align-3/baseline-align-test with body { writing-mode: vertical-rl } or vertical-lr, too. But, Firefox actually seems to use the central baseline for those cases when loading the testcase in a vertical writing-mode (and we use the central baseline for most of the cases there, in fact), rather than using the perhaps-proposed block-end margin-edge. So: it seems we're not interoperable on that particular point.

(CC @jfkthame & @MatsPalmgren who may have thoughts on this, too)

@tabatkins
Copy link
Member

Oh phew, yes, definitely a writing-mode-aware edge, sorry about that.

Question, tho: line-under, or block-end? @fantasai, you know the appropriateness of line-under usage better than me.

@fantasai
Copy link
Collaborator

@tabatkins block-end of the scroll container, because we're compensating for the fact the line we would be aligning to is scrolled off-screen, potentially (or at least, I think that's the "logic" being applied here)?

@dholbert Firefox's handling of baselines in vertical writing modes is extremely broken in general. Not really useful as a reference implementation. :/

@fantasai fantasai added Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. and removed Needs Edits labels Jan 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closed Accepted by Editor Discretion Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. CSS2 css-align-3 Current Work
Projects
None yet
Development

No branches or pull requests

7 participants