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-grid] Fix line name positions after auto repeat with no line names #19661

Merged
merged 1 commit into from Oct 23, 2019

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Oct 13, 2019

When serializing grid-template-rows/columns of a grid container, we need
to handle auto repeat() specially in order to insert the line names at
the correct places.

Before this patch, this was skipped for indices before the insertion
point of the auto repeat, and in case the auto repeat had no line name.
The latter logic was wrong, if there is an auto repeat we still need the
special code after the insertion point, even if it has no line names.
The proper condition to check is whether there is no auto repeat.

The patch also avoids a 2nd call to GridAutoRepeatRows/Columns since we
already have the value in a variable.

BUG=1011329

TEST=external/wpt/css/css-grid/parsing/grid-template-columns-computed-nogrid.html
TEST=external/wpt/css/css-grid/parsing/grid-template-columns-computed.html
TEST=external/wpt/css/css-grid/parsing/grid-template-rows-computed-nogrid.html
TEST=external/wpt/css/css-grid/parsing/grid-template-rows-computed-withcontent.html
TEST=external/wpt/css/css-grid/parsing/grid-template-rows-computed.html

There are some test failures because integer repeat() is still expanded
at computed-value time (http://crbug.com/989004).

Change-Id: I16d06275384ab8c7866b4981ba8dcc665258b29d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1857125
Reviewed-by: Manuel Rego <rego@igalia.com>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/master@{#708661}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

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

Already reviewed downstream.

When serializing grid-template-rows/columns of a grid container, we need
to handle auto repeat() specially in order to insert the line names at
the correct places.

Before this patch, this was skipped for indices before the insertion
point of the auto repeat, and in case the auto repeat had no line name.
The latter logic was wrong, if there is an auto repeat we still need the
special code after the insertion point, even if it has no line names.
The proper condition to check is whether there is no auto repeat.

The patch also avoids a 2nd call to GridAutoRepeatRows/Columns since we
already have the value in a variable.

BUG=1011329

TEST=external/wpt/css/css-grid/parsing/grid-template-columns-computed-nogrid.html
TEST=external/wpt/css/css-grid/parsing/grid-template-columns-computed.html
TEST=external/wpt/css/css-grid/parsing/grid-template-rows-computed-nogrid.html
TEST=external/wpt/css/css-grid/parsing/grid-template-rows-computed-withcontent.html
TEST=external/wpt/css/css-grid/parsing/grid-template-rows-computed.html

There are some test failures because integer repeat() is still expanded
at computed-value time (http://crbug.com/989004).

Change-Id: I16d06275384ab8c7866b4981ba8dcc665258b29d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1857125
Reviewed-by: Manuel Rego <rego@igalia.com>
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Cr-Commit-Position: refs/heads/master@{#708661}
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.

None yet

4 participants