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.

Sign up for GitHub

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

Jump to bottom

InteractionRegions are missing for some display: table elements #14390

Merged
merged 1 commit into from May 26, 2023
Merged

InteractionRegions are missing for some display: table elements #14390

merged 1 commit into from May 26, 2023

Conversation

etiennesegonzac
Copy link
Contributor

@etiennesegonzac etiennesegonzac commented May 26, 2023 •
edited by webkit-early-warning-system

fde1853

InteractionRegions are missing for some `display: table` elements
https://bugs.webkit.org/show_bug.cgi?id=257379
<rdar://109538261>

Reviewed by Tim Horton.

Don't skip the child renderer's InteractionRegion if the link parent is
`display: table`, like we do for inline links.

* Source/WebCore/page/InteractionRegion.cpp:
(WebCore::interactionRegionForRenderedRegion):
Check if the matched renderer `isDisplayTableOrTablePart()` before
bailing out.

* LayoutTests/interaction-region/display-table-expected.txt: Added.
* LayoutTests/interaction-region/display-table.html: Added.
Add a test.

* LayoutTests/interaction-region/layer-tree-expected.txt:
Test update.

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

b04ffac

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@etiennesegonzac etiennesegonzac self-assigned this May 26, 2023
@etiennesegonzac etiennesegonzac added the Layout and Rendering For bugs with layout and rendering of Web pages. label May 26, 2023
@webkit-early-warning-system
Copy link
Collaborator

EWS run on current version of this PR (hash b04ffac)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

nt1m
nt1m reviewed May 26, 2023
View reviewed changes
Source/WebCore/page/InteractionRegion.cpp
@@ -258,7 +258,7 @@ std::optional<InteractionRegion> interactionRegionForRenderedRegion(RenderObject
bool isInlineNonBlock = renderer.isInline() && !renderer.isReplacedOrInlineBlock();

// The parent will get its own InteractionRegion.
if (!isOriginalMatch && !isInlineNonBlock)
if (!isOriginalMatch && !isInlineNonBlock && !renderer.style().isDisplayTableOrTablePart())
Copy link
Member

Choose a reason for hiding this comment

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

I think on the web, some people use display: table or similar to do clearfix hacks, not sure that affects the code you're writing, but worth noting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No impact here, but happy to know one of the reasons for this setup!

@etiennesegonzac etiennesegonzac added the merge-queue Applied to send a pull request to merge-queue label May 26, 2023
@etiennesegonzac
InteractionRegions are missing for some display: table elements
fde1853 
https://bugs.webkit.org/show_bug.cgi?id=257379
<rdar://109538261>

Reviewed by Tim Horton.

Don't skip the child renderer's InteractionRegion if the link parent is
`display: table`, like we do for inline links.

* Source/WebCore/page/InteractionRegion.cpp:
(WebCore::interactionRegionForRenderedRegion):
Check if the matched renderer `isDisplayTableOrTablePart()` before
bailing out.

* LayoutTests/interaction-region/display-table-expected.txt: Added.
* LayoutTests/interaction-region/display-table.html: Added.
Add a test.

* LayoutTests/interaction-region/layer-tree-expected.txt:
Test update.

Canonical link: https://commits.webkit.org/264598@main
@webkit-commit-queue
Copy link
Collaborator

Committed 264598@main (fde1853): https://commits.webkit.org/264598@main

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

@webkit-commit-queue webkit-commit-queue merged commit fde1853 into WebKit:main May 26, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nt1m nt1m nt1m left review comments

@hortont424 hortont424 hortont424 approved these changes

@cdumez cdumez Awaiting requested review from cdumez cdumez is a code owner

@megangardner megangardner Awaiting requested review from megangardner

Assignees

@etiennesegonzac etiennesegonzac

Labels
Layout and Rendering For bugs with layout and rendering of Web pages.
Projects
None yet
Milestone
No milestone
5 participants
@etiennesegonzac @webkit-early-warning-system @webkit-commit-queue @hortont424 @nt1m