Skip to content

[Grid] Fix grid-container-ignores-first-letter-001 #47312

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

Conversation

sammygill
Copy link
Contributor

@sammygill sammygill commented Jun 27, 2025

47b3695

[Grid] Fix grid-container-ignores-first-letter-001
https://bugs.webkit.org/show_bug.cgi?id=295114
rdar://154504582

Reviewed by Alan Baradlay.

The first-letter text comes from the "first formatted line of its
originating element." Finding the first formatted line of a block
container:
  - Comes from the inline level content of the first line box if the
    block container establishes an inline formatting context.
  - If it contains block-level content, then it comes from the first
    formatted line of the first in-flow block-level child. If no such
    line exists, then there is no formatted line.

The flexbox and grid specs state that they do not contribute a first
formatted line or letter.

In theory, finding the first formatted line requires performing layout,
but we construct first letter renderers and associated structures during
render tree building/updating, which occurs before.
RenderBlock::firstLetterAndContainer attempts to implement a heuristic
in which it finds the block container that has the first letter style
and then digs into its content to find the renderer that would be
associated with that style. During this process, if we hit a flexbox or a
grid, we seem to just assign firstLetter to the next sibling and continue
the heuristic. Instead, we should *not* return a first letter container
and renderer since there should be no first formatted line and, as a
result, no first letter.

* LayoutTests/css3/flexbox/flexbox-ignore-container-firstLetter-expected.txt:
* LayoutTests/css3/flexbox/flexbox-ignore-container-firstLetter.html:
* LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter.html: Removed.
This test seems to be a duplicate of the one contained in the grid WPT
directory (grid-container-ignores-first-letter-001).

* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flexbox-ignores-first-letter.html
This is the same as the grid test but for flexbox.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flexbox-ignores-first-letter-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flexbox-ignores-first-letter.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-model/grid-container-ignores-first-letter-001-expected.txt:
* Source/WebCore/rendering/RenderBlock.cpp:

* LayoutTests/css3/flexbox/flexbox-ignore-container-firstLetter-expected.txt
* LayoutTests/css3/flexbox/flexbox-ignore-container-firstLetter.html
Flexboxes to not generate a first formatted line or a first letter for
their ancestors. As a result, the container does not have a first
formatted line/letter since the first in flow child does not.

Canonical link: https://commits.webkit.org/296797@main

8d52c37

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@sammygill sammygill self-assigned this Jun 27, 2025
@sammygill sammygill added the Layout and Rendering For bugs with layout and rendering of Web pages. label Jun 27, 2025
@sammygill sammygill requested a review from alanbaradlay June 27, 2025 18:09
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 27, 2025
@sammygill sammygill removed the merging-blocked Applied to prevent a change from being merged label Jun 27, 2025
@sammygill sammygill force-pushed the eng/Grid-Fix-grid-container-ignores-first-letter-001 branch from 783c42a to 8d52c37 Compare June 27, 2025 22:12
@sammygill sammygill added the merge-queue Applied to send a pull request to merge-queue label Jun 30, 2025
https://bugs.webkit.org/show_bug.cgi?id=295114
rdar://154504582

Reviewed by Alan Baradlay.

The first-letter text comes from the "first formatted line of its
originating element." Finding the first formatted line of a block
container:
  - Comes from the inline level content of the first line box if the
    block container establishes an inline formatting context.
  - If it contains block-level content, then it comes from the first
    formatted line of the first in-flow block-level child. If no such
    line exists, then there is no formatted line.

The flexbox and grid specs state that they do not contribute a first
formatted line or letter.

In theory, finding the first formatted line requires performing layout,
but we construct first letter renderers and associated structures during
render tree building/updating, which occurs before.
RenderBlock::firstLetterAndContainer attempts to implement a heuristic
in which it finds the block container that has the first letter style
and then digs into its content to find the renderer that would be
associated with that style. During this process, if we hit a flexbox or a
grid, we seem to just assign firstLetter to the next sibling and continue
the heuristic. Instead, we should *not* return a first letter container
and renderer since there should be no first formatted line and, as a
result, no first letter.

* LayoutTests/css3/flexbox/flexbox-ignore-container-firstLetter-expected.txt:
* LayoutTests/css3/flexbox/flexbox-ignore-container-firstLetter.html:
* LayoutTests/fast/css-grid-layout/grid-container-ignore-first-letter.html: Removed.
This test seems to be a duplicate of the one contained in the grid WPT
directory (grid-container-ignores-first-letter-001).

* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flexbox-ignores-first-letter.html
This is the same as the grid test but for flexbox.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flexbox-ignores-first-letter-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/flexbox-ignores-first-letter.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-model/grid-container-ignores-first-letter-001-expected.txt:
* Source/WebCore/rendering/RenderBlock.cpp:

* LayoutTests/css3/flexbox/flexbox-ignore-container-firstLetter-expected.txt
* LayoutTests/css3/flexbox/flexbox-ignore-container-firstLetter.html
Flexboxes to not generate a first formatted line or a first letter for
their ancestors. As a result, the container does not have a first
formatted line/letter since the first in flow child does not.

Canonical link: https://commits.webkit.org/296797@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Grid-Fix-grid-container-ignores-first-letter-001 branch from 8d52c37 to 47b3695 Compare June 30, 2025 16:00
@webkit-commit-queue
Copy link
Collaborator

Committed 296797@main (47b3695): https://commits.webkit.org/296797@main

Reviewed commits have been landed. Closing PR #47312 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 47b3695 into WebKit:main Jun 30, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jun 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Layout and Rendering For bugs with layout and rendering of Web pages.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants