Summary: | getBoundingClientRect() returns wrong value for tr, td and its descendants for a vertical table | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yuki Sekiguchi <yuki.sekiguchi> | ||||||||||||||||||
Component: | Layout and Rendering | Assignee: | Yuki Sekiguchi <yuki.sekiguchi> | ||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||
Severity: | Normal | CC: | ahmad.saleem792, bfulgham, buildbot, changseok, darin, esprehn+autocc, ews-watchlist, glenn, hyatt, koivisto, kondapallykalyan, mjs, pdr, simon.fraser, skoji, webkit-bug-importer, ysuzuki, zalan | ||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||
Version: | WebKit Local Build | ||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||
Attachments: |
|
Created attachment 306907 [details]
Patch
Comment on attachment 306907 [details] Patch Attachment 306907 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3523775 New failing tests: fast/writing-mode/vertical-align-table-baseline.html fast/table/040-vertical.html fast/overflow/overflow-rtl-vertical.html fast/table/rowspan-paint-order-vertical.html fast/table/table-display-types-vertical.html fast/table/border-collapsing/002-vertical.html fast/table/growCellForImageQuirk-vertical.html fast/table/038-vertical.html fast/css/h1-in-section-elements.html fast/table/border-collapsing/003-vertical.html fast/table/border-collapsing/001-vertical.html fast/table/028-vertical.html fast/table/auto-with-percent-height-vertical.html fast/table/border-collapsing/rtl-border-collapsing-vertical.html fast/table/027-vertical.html Created attachment 306908 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Created attachment 306960 [details]
Patch
Comment on attachment 306960 [details]
Patch
Seems like a good idea to fix this bug, unfortunately, this version of the patch no longer applies. Seems like just one of the test diffs that has an issue, so shouldn't be too hard to fix up.
Created attachment 400780 [details]
Patch
Created attachment 400801 [details]
Patch
Created attachment 400809 [details]
Patch
Created attachment 400885 [details]
Patch
Hi Maciej, I rebased my patch onto the latest WebKit and fixed LayoutTest errors. Could you review it again? Comment on attachment 400885 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=400885&action=review This looks good to me. Not sure I’m sufficiently expert to be the reviewer though. > Source/WebCore/rendering/RenderTableCell.cpp:347 > + if (parent()) { > + RenderElement* containerOfRow = container.container(); > + offset -= parent()->offsetFromContainer(*containerOfRow, point); > + } What guarantees that container.container() is non-null? Tiny point: I don’t think the local variable is doing a lot that is helpful here. I think that writing *container.container() is probably OK without explicitly documenting that it’s the container of the row. After all, we are not documenting what parent is (the section, maybe?). > Source/WebCore/rendering/RenderTableSection.cpp:562 > + rowRenderer->setLogicalLeft(0_lu); > + rowRenderer->setLogicalTop(m_rowPos[r]); > rowRenderer->setLogicalWidth(logicalWidth()); > rowRenderer->setLogicalHeight(m_rowPos[r + 1] - m_rowPos[r] - vspacing); Is there a setLogical function that takes a LayoutRect? If so, maybe we’d want to use it here? I am still able to reproduce this bug in Safari 16 and Safari Technology Preview 154 using attached "Reproduced Content" while both Chrome Canary 108 and Firefox Nightly 107 show "success' in the test case. Thanks! Committed 254918@main (1e62e8aa2236): <https://commits.webkit.org/254918@main> Reviewed commits have been landed. Closing PR #4583 and removing active labels. |
Created attachment 306902 [details] Reproduced content getBoundingClientRect() returns wrong value for tr, td and its descendants for a vertical table In the attached content, the rect for tds are out side of the td which is its parent. Inspector highlights wrong rects for tr, td and its descendants as well.