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-overflow-4] Draft spec for continue: collapse (#7708) #10816

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

andreubotella
Copy link
Member

No description provided.

@bfgeek
Copy link

bfgeek commented Sep 17, 2024

@emilio - Does this look reasonable to you?

@andreubotella
Copy link
Member Author

Before now, this PR allowed clamping between any two blocks, to make it closer to the continue: discard behavior, but in hallway conversations in TPAC we agreed that having it clamp to the last line before clamp, even if there are lineless boxes after it that will still fit, was a better behavior. I just updated the spec text to reflect this.

@tabatkins
Copy link
Member

but in hallway conversations in TPAC we agreed that having it clamp to the last line before clamp, even if there are lineless boxes after it that will still fit, was a better behavior

In particular, the argument was that auto clamping should be equivalent to the largest <integer> clamp that avoids overflow; it's weird if auto actually lets you include an amount of content between two <integer> values, and thus can't be reproduced directly.

moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 19, 2024
This matches the proposal in
w3c/csswg-drafts#10816, and creates much better
behavior.

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

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1791226
gecko-commit: c6ce92301ca11292a5dd25f07a80243592c94e5a
gecko-reviewers: layout-reviewers, dshin
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Oct 19, 2024
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 21, 2024
This matches the proposal in
w3c/csswg-drafts#10816, and creates much better
behavior.

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

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1791226
gecko-commit: cf9f54eef33465d089f850a55da5734a911fdac4
gecko-reviewers: layout-reviewers, dshin
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this pull request Oct 21, 2024
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Oct 21, 2024
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 21, 2024
This matches the proposal in
w3c/csswg-drafts#10816, and creates much better
behavior.

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

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1791226
gecko-commit: cf9f54eef33465d089f850a55da5734a911fdac4
gecko-reviewers: layout-reviewers, dshin
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 22, 2024
This matches the proposal in
w3c/csswg-drafts#10816, and creates much better
behavior.

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

UltraBlame original commit: c6ce92301ca11292a5dd25f07a80243592c94e5a
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 22, 2024
This matches the proposal in
w3c/csswg-drafts#10816, and creates much better
behavior.

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

UltraBlame original commit: cf9f54eef33465d089f850a55da5734a911fdac4
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 22, 2024
This matches the proposal in
w3c/csswg-drafts#10816, and creates much better
behavior.

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

UltraBlame original commit: c6ce92301ca11292a5dd25f07a80243592c94e5a
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 22, 2024
This matches the proposal in
w3c/csswg-drafts#10816, and creates much better
behavior.

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

UltraBlame original commit: cf9f54eef33465d089f850a55da5734a911fdac4
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 22, 2024
This matches the proposal in
w3c/csswg-drafts#10816, and creates much better
behavior.

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

UltraBlame original commit: c6ce92301ca11292a5dd25f07a80243592c94e5a
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 22, 2024
This matches the proposal in
w3c/csswg-drafts#10816, and creates much better
behavior.

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

UltraBlame original commit: cf9f54eef33465d089f850a55da5734a911fdac4
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this pull request Oct 23, 2024
Copy link
Collaborator

@frivoal frivoal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems mostly good. I've made a few small suggestion for normative changes, and more suggestions for editorial changes (most of which are about source formatting).

Most of the remaining issues are orthogonal to this PR, so I think we can merge first and keep working on those later, as soon as the comments on this PR are addressed.

that would receive subsequent content,
then the content displaced by the <a>block overflow ellipsis</a>
must be pushed to that <a>fragmentation container</a>.
must be pushed to that <a>fragmentation container</a>. If it is placed before a [=clamp point=],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Editorial:

Suggested change
must be pushed to that <a>fragmentation container</a>. If it is placed before a [=clamp point=],
must be pushed to that <a>fragmentation container</a>.
If it is placed before a [=clamp point=],

Reason: this spec uses semantic line-breaks, and I'd prefer to stay consistent with that. See https://rhodesmill.org/brandon/2012/one-sentence-per-line/

that would receive subsequent content,
then the content displaced by the <a>block overflow ellipsis</a>
must be pushed to that <a>fragmentation container</a>.
must be pushed to that <a>fragmentation container</a>. If it is placed before a [=clamp point=],
then the displaced content must be pushed to the remainder of the [=inline formatting context=].
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok with that, but I think this has been controversial, with some advocating that the displaced content be dropped / invisible instead, in order to avoid that inserting the ellipsis would cause an extra layout pass. Should we open a separate issue to discuss that aspect?
Or maybe this is unavoidable due to bidi processing of the ellipsis, which means we have to relayout anyway?

@@ -844,7 +849,7 @@ Limiting Visible Lines: the 'line-clamp' shorthand property</h3>
<dt><dfn><<integer [1,∞]>></dfn>
</dt><dfn><'block-ellipsis'></dfn>
<dd>
Sets 'continue' to ''discard''
Sets 'continue' to ''collapse''
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Sets 'continue' to ''collapse''
Issue(7708): Whether this should imply ''discard'' or ''collapse'' is an unresolved question.
Sets 'continue' to ''collapse''

<var>N</var> is the computed value of 'max-lines':

- If the box is a [=fragmentation container=] that captures [=region breaks=],
a <a>region break</a> or
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
a <a>region break</a> or
a <a>region break</a>

Comment on lines +962 to +963
If the value of 'max-lines' is not <dfn for=max-lines dfn-type=value>none</dfn>, then, where
<var>N</var> is the computed value of 'max-lines':
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Editorial:

Suggested change
If the value of 'max-lines' is not <dfn for=max-lines dfn-type=value>none</dfn>, then, where
<var>N</var> is the computed value of 'max-lines':
If the value of 'max-lines' is not <dfn for=max-lines dfn-type=value>none</dfn>,
then, given a [=computed value=] of <var>N</var>:

Comment on lines +1172 to +1173
- A point immediately after a [=line box=] in the line-clamp container's [=block formatting
context=].
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Editorial

Suggested change
- A point immediately after a [=line box=] in the line-clamp container's [=block formatting
context=].
- A point immediately after a [=line box=]
in the line-clamp container's [=block formatting context=].

See https://rhodesmill.org/brandon/2012/one-sentence-per-line/

Comment on lines +1174 to +1175
- The end of the line-clamp container, if there are any boxes after the last [=inline formatting
context=] in the line-clamp container's [=block formatting context=].
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Editorial

Suggested change
- The end of the line-clamp container, if there are any boxes after the last [=inline formatting
context=] in the line-clamp container's [=block formatting context=].
- The end of the line-clamp container,
if there are any boxes after the last [=inline formatting context=]
in the line-clamp container's [=block formatting context=].

See https://rhodesmill.org/brandon/2012/one-sentence-per-line/

Comment on lines +1185 to +1189
Any boxes in a [=line-clamp container=] that follow its [=clamp point=] in the box tree, as well
as any [=line boxes=] that follow it inside an [=inline formatting context=], will be invisible
(behaving like ''visibility: hidden''). This includes [=independent formatting contexts=] as
well as [=out-of-flow=] boxes, and all of their descendants. Any overflow such boxes and line
boxes might have is always counted as [=ink overflow=] rather than [=scrollable overflow=].
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Editorial

Suggested change
Any boxes in a [=line-clamp container=] that follow its [=clamp point=] in the box tree, as well
as any [=line boxes=] that follow it inside an [=inline formatting context=], will be invisible
(behaving like ''visibility: hidden''). This includes [=independent formatting contexts=] as
well as [=out-of-flow=] boxes, and all of their descendants. Any overflow such boxes and line
boxes might have is always counted as [=ink overflow=] rather than [=scrollable overflow=].
Any boxes in a [=line-clamp container=] that follow its [=clamp point=] in the box tree,
as well as any [=line boxes=] that follow it inside an [=inline formatting context=],
will be invisible (behaving like ''visibility: hidden'').
This includes [=independent formatting contexts=]
as well as [=out-of-flow=] boxes,
and all of their descendants.
Any overflow such boxes and line boxes might have
is always counted as [=ink overflow=] rather than [=scrollable overflow=].

See https://rhodesmill.org/brandon/2012/one-sentence-per-line/

Comment on lines +1177 to +1183
If the [=line-clamp container=]'s [=block formatting context root=] has a [=computed value=] of
'max-lines' other than ''max-lines/none'', then that property will determine the [=clamp point=].
Otherwise, the clamp point will be set to the last possible clamp point such that, for it and
all previous possible clamp points, the line-clamp container's [=automatic block size=] (as
determined below) is not greater than the [=block size=] the box would have if its automatic
block size were infinite; or if that is not the case for any clamp points, to the first clamp
point in the block formatting context.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Editorial:

Suggested change
If the [=line-clamp container=]'s [=block formatting context root=] has a [=computed value=] of
'max-lines' other than ''max-lines/none'', then that property will determine the [=clamp point=].
Otherwise, the clamp point will be set to the last possible clamp point such that, for it and
all previous possible clamp points, the line-clamp container's [=automatic block size=] (as
determined below) is not greater than the [=block size=] the box would have if its automatic
block size were infinite; or if that is not the case for any clamp points, to the first clamp
point in the block formatting context.
If the [=line-clamp container=]'s [=block formatting context root=]
has a [=computed value=] of 'max-lines' other than ''max-lines/none'',
then that property will determine the [=clamp point=].
Otherwise, the clamp point will be set to the last possible clamp point
such that, for it and all previous possible clamp points,
the line-clamp container's [=automatic block size=]
(as determined below)
is not greater than the [=block size=] the box would have
if its automatic block size were infinite;
or if that is not the case for any possible clamp points,
to the first possible clamp point in the block formatting context.

See https://rhodesmill.org/brandon/2012/one-sentence-per-line/

I also added the word "possible" before "clamp point" in the last two lines.

Comment on lines +715 to +718
This property only affects a line box if it immediately precedes
eiher a [=region break=] or a [=clamp point=] in the [=block formatting context=].
If the [=clamp point=] is placed at the end of the [=line-clamp container=], then the line will
not be affected.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Editorial

Suggested change
This property only affects a line box if it immediately precedes
eiher a [=region break=] or a [=clamp point=] in the [=block formatting context=].
If the [=clamp point=] is placed at the end of the [=line-clamp container=], then the line will
not be affected.
This property only affects a line box
if it immediately precedes eiher a [=region break=]
or a [=clamp point=] in the [=block formatting context=].
If the [=clamp point=] is placed at the end of the [=line-clamp container=],
then the line will not be affected.

Reason: this spec uses semantic line-breaks, and I'd prefer to stay consistent with that. See https://rhodesmill.org/brandon/2012/one-sentence-per-line/

@frivoal
Copy link
Collaborator

frivoal commented Nov 4, 2024

@tabatkins , for your comment in #10816 (comment), I'd suggest following up in #10868, because that aspect exists before this PR. The text of the spec may need to change depending on where we land on that issue (and others), but this PR isn't changing it, so I think we should take one problem at a time.

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

Successfully merging this pull request may close these issues.

4 participants