148690 2015-09-01 14:51:46 -0700 http/tests/navigation/anchor-frames-same-origin.html is flaky 2024-07-08 16:36:14 -0700 1 1 1 Unclassified WebKit Layout and Rendering WebKit Nightly Build Unspecified Unspecified REOPENED https://bugs.webkit.org/show_bug.cgi?id=148615 https://bugs.webkit.org/show_bug.cgi?id=191284 P2 Normal --- 1 cdumez zalan ahmad.saleem792 ap cdumez simon.fraser oldest_to_newest 1122657 0 cdumez 2015-09-01 14:51:46 -0700 http/tests/navigation/anchor-frames-same-origin.html passes when run after http/tests/navigation/anchor-frames-cross-origin.html (case a) but fails when run on its own (case b). The test uses the following frame structure: - main Frame -- child Frame (100x100px) --- grand-child frame (8000x8000px) ----- anchor is at (0, 2000) px When debugging it, I found that when we call FrameView::maintainScrollPositionAtAnchor(), we are in the following state: - case a: Layout is done (renderView->needsLayout() returns false) so we call FrameView::scrollToAnchor() right away. - case b: We need a layout, so we call layout() and only call FrameView::scrollToAnchor() after layout is done in performPostLayoutTasks(). scrollToAnchor() calls RenderLayer::scrollRectToVisible(rect) with the visible rect of the anchor we need to scroll to. This is the code that will actually scroll the view if needed and well as its parents. The logic looks like this: === frameView.setScrollPosition(IntPoint(xOffset, yOffset)); if (frameView.safeToPropagateScrollToParent()) { parentLayer = ownerElement->renderer()->enclosingLayer(); // FIXME: This doesn't correctly convert the rect to // absolute coordinates in the parent. newRect.setX(rect.x() - frameView.scrollX() + frameView.x()); newRect.setY(rect.y() - frameView.scrollY() + frameView.y()); fprintf(stderr, "safeToPropagateScrollToParent, newRect: %d, %d, frameView.scrollY(): %d\n", newRect.x().toInt(), newRect.y().toInt(), frameView.scrollY()); } === In case a, when we compute the parent's new Rect, we end up with 2346px as height because frameView.scrollY() returns 0 (as expected given that the content height is smaller than the frame height). However, in case b, the new Rect has a height of 0 because frameView.scrollY() returns 2008. For some reason, in case b, FrameView::maximumScrollPosition() returns [77, 2142] instead of [0, 0] as in case a. Render tree in case a when scrollToAnchor() is called: (R)elative/A(B)solute/Fi(X)ed/Stick(Y) positioned, (O)verflow clipping, (A)nonymous, (G)enerated, (F)loating, has(L)ayer, (C)omposited, (D)irty layout, Dirty (S)tyle ---G-L- --* RenderView (0.00, 0.00) (8000.00, 8000.00) renderer->(0x11629fbe0) -----L- -- HTML RenderBlock (0.00, 0.00) (8000.00, 2034.00) renderer->(0x1163bdc60) node->(0x1163e52a0) ------- -- BODY RenderBody (8.00, 8.00) (7984.00, 2018.00) renderer->(0x1163bdd10) node->(0x116335a80) ------- -- DIV RenderBlock (0.00, 0.00) (7984.00, 2000.00) renderer->(0x1163bddc0) node->(0x116335ae0) --AG--- -- RenderBlock (0.00, 2000.00) (7984.00, 18.00) renderer->(0x1163bde70) ------- -- RootInlineBox (0.00, 0.00) (265.88, 18.00) (0x1163f1b40) renderer->(0x1163bde70) ------- -- InlineTextBox (0.00, 0.00) (261.88, 18.00) (0x116331160) run(0, 39) "This is an anchor point named "anchor1"" ------- -- InlineTextBox (261.88, 0.00) (4.00, 18.00) (0x1163311b8) run(0, 1) "." ------- -- A RenderInline renderer->(0x116343600) node->(0x1163ed5a0) ------- -- #text RenderText renderer->(0x1163a1dc0) node->(0x11634bea0) length->(39) "This is an anchor point named "anchor1"" ------- -- #text RenderText renderer->(0x1163a1f20) node->(0x11634bee8) length->(4) ".\n\n\n" ==== Render tree in case b when scrollToAnchor is called: (R)elative/A(B)solute/Fi(X)ed/Stick(Y) positioned, (O)verflow clipping, (A)nonymous, (G)enerated, (F)loating, has(L)ayer, (C)omposited, (D)irty layout, Dirty (S)tyle ---G-L- --* RenderView (0.00, 0.00) (0.00, 0.00) renderer->(0x1152ecbe0) -----L- -- HTML RenderBlock (0.00, 0.00) (0.00, 2142.00) renderer->(0x1153c6840) node->(0x1153ac4e0) ------- -- BODY RenderBody (8.00, 8.00) (0.00, 2126.00) renderer->(0x1153c68f0) node->(0x1153ac5a0) ------- -- DIV RenderBlock (0.00, 0.00) (0.00, 2000.00) renderer->(0x1153c69a0) node->(0x1153ac600) --AG--- -- RenderBlock (0.00, 2000.00) (0.00, 126.00) renderer->(0x1153c6a50) ------- -- RootInlineBox (0.00, 0.00) (28.45, 18.00) (0x1153f1aa0) renderer->(0x1153c6a50) ------- -- InlineTextBox (0.00, 0.00) (28.45, 18.00) (0x1153460b0) run(0, 4) "This" ------- -- RootInlineBox (0.00, 18.00) (10.67, 18.00) (0x1153f1b40) renderer->(0x1153c6a50) ------- -- InlineTextBox (0.00, 18.00) (10.67, 18.00) (0x115346108) run(5, 7) "is" ------- -- RootInlineBox (0.00, 36.00) (15.10, 18.00) (0x1153f1be0) renderer->(0x1153c6a50) ------- -- InlineTextBox (0.00, 36.00) (15.10, 18.00) (0x115346160) run(8, 10) "an" ------- -- RootInlineBox (0.00, 54.00) (43.53, 18.00) (0x1153f1c80) renderer->(0x1153c6a50) ------- -- InlineTextBox (0.00, 54.00) (43.53, 18.00) (0x1153461b8) run(11, 17) "anchor" ------- -- RootInlineBox (0.00, 72.00) (32.89, 18.00) (0x1153f1d20) renderer->(0x1153c6a50) ------- -- InlineTextBox (0.00, 72.00) (32.89, 18.00) (0x115346210) run(18, 23) "point" ------- -- RootInlineBox (0.00, 90.00) (42.65, 18.00) (0x1153f1dc0) renderer->(0x1153c6a50) ------- -- InlineTextBox (0.00, 90.00) (42.65, 18.00) (0x115346268) run(24, 29) "named" ------- -- RootInlineBox (0.00, 108.00) (68.59, 18.00) (0x1153f1e60) renderer->(0x1153c6a50) ------- -- InlineTextBox (0.00, 108.00) (64.59, 18.00) (0x1153462c0) run(30, 39) ""anchor1"" ------- -- InlineTextBox (64.59, 108.00) (4.00, 18.00) (0x115346318) run(0, 1) "." ------- -- A RenderInline renderer->(0x11534c060) node->(0x1153ede88) ------- -- #text RenderText renderer->(0x115380f20) node->(0x115355dc8) length->(39) "This is an anchor point named "anchor1"" ------- -- #text RenderText renderer->(0x115380f78) node->(0x115355e10) length->(4) ".\n\n\n" 1122660 1 cdumez 2015-09-01 14:56:24 -0700 It looks like it fails when run on its own on WebKit2 as well. 1122661 2 cdumez 2015-09-01 15:01:10 -0700 Passes: Tools/Scripts/run-webkit-tests --release http/tests/navigation/anchor-frames-cross-origin.html http/tests/navigation/anchor-frames-same-origin.html Fails: Tools/Scripts/run-webkit-tests --release http/tests/navigation/anchor-frames-same-origin.html 1122767 3 260408 cdumez 2015-09-01 22:16:32 -0700 Created attachment 260408 Temporary workaround 1122768 4 260408 cdumez 2015-09-01 22:17:18 -0700 Comment on attachment 260408 Temporary workaround Zalan, what do you think? Can we land this in the mean time? 1122840 5 cdumez 2015-09-02 10:06:39 -0700 Committed r189250: <http://trac.webkit.org/changeset/189250> 1122841 6 cdumez 2015-09-02 10:07:18 -0700 Reopening as it is a temporary workaround. 2045198 7 ahmad.saleem792 2024-07-08 16:36:14 -0700 We still have this workaround - https://searchfox.org/wubkat/rev/df1e46cebe93c020392645e94f67469843cd8063/LayoutTests/http/tests/navigation/resources/grandchild-with-anchor.html#8 260408 2015-09-01 22:16:32 -0700 2015-09-02 08:22:47 -0700 Temporary workaround bug-148690-20150901221616.patch text/plain 1945 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMTg5MjM5CmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9DaGFu Z2VMb2cgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKaW5kZXggNDBkM2Q1ZWU1MGNkNWE3YjRhYTQ0 NzZhOTcwZDQ4ZWFiZDY2OWRhMS4uMTE0MDVlYTBkMThiM2UwZGNmNDBlNDQ3YTJiNjU2NzJlMDY0 MTMyZCAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCisrKyBiL0xheW91dFRlc3Rz L0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDE1LTA5LTAxICBDaHJpcyBEdW1leiAgPGNk dW1lekBhcHBsZS5jb20+CisKKyAgICAgICAgaHR0cC90ZXN0cy9uYXZpZ2F0aW9uL2FuY2hvci1m cmFtZXMtc2FtZS1vcmlnaW4uaHRtbCBpcyBmbGFreQorICAgICAgICBodHRwczovL2J1Z3Mud2Vi a2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTQ4NjkwCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9C T0RZIChPT1BTISkuCisKKyAgICAgICAgVGhpcyBpcyBhIHRlbXBvcmFyeSB3b3JrYXJvdW5kIGZv ciBCdWcgMTQ4NjkwLCB1bnRpbCB3ZSBoYXZlIHRpbWUKKyAgICAgICAgdG8gaW52ZXN0aWdhdGUg d2h5IHNjcm9sbCBwcm9wYWdhdGlvbiBkb2VzIG5vdCB3b3JrIHByb3Blcmx5IGlmCisgICAgICAg IHRoZSBmcmFtZSBpcyBub3QgYWxyZWFkeSBsYXllZCBvdXQgd2hlbiB3ZSBzY3JvbGwgdG8gdGhl IGFuY2hvci4KKworICAgICAgICBUaGlzIGNoYW5nZSB1cGRhdGVzIHRoZSB0ZXN0IHRvIGZvcmNl IGEgbGF5b3V0IGluIHRoZSBncmFuZGNoaWxkCisgICAgICAgIGZyYW1lIGJlZm9yZSB3ZSBzY3Jv bGwgdG8gdGhlIGFuY2hvci4gV2l0aCB0aGlzIGNoYW5nZSwgdGhlIHRlc3QKKyAgICAgICAgaXMg Y29uc2lzdGVudGx5IHBhc3Npbmcgd2hlbiBydW4gb24gaXRzIG93biBvciBhZnRlciBvdGhlcnMu CisKKyAgICAgICAgKiBodHRwL3Rlc3RzL25hdmlnYXRpb24vcmVzb3VyY2VzL2dyYW5kY2hpbGQt d2l0aC1hbmNob3IuaHRtbDoKKwogMjAxNS0wOS0wMSAgSmVyZW15IEpvbmVzICA8amVyZW15akBh cHBsZS5jb20+CiAKICAgICAgICAgVW5yZXZpZXdlZCwgTGF5b3V0IFRlc3QgaHR0cC90ZXN0cy9p bnNwZWN0b3IvZG9tL2Rpc2Nvbm5lY3QtZG9tLXRyZWUtYWZ0ZXItbWFpbi1mcmFtZS1uYXZpZ2F0 aW9uLmh0bWwgaXMgZmFpbGluZwpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvaHR0cC90ZXN0cy9u YXZpZ2F0aW9uL3Jlc291cmNlcy9ncmFuZGNoaWxkLXdpdGgtYW5jaG9yLmh0bWwgYi9MYXlvdXRU ZXN0cy9odHRwL3Rlc3RzL25hdmlnYXRpb24vcmVzb3VyY2VzL2dyYW5kY2hpbGQtd2l0aC1hbmNo b3IuaHRtbAppbmRleCBiOGI3OWU0OWY3YzU2NzE4MDk1MzRkMzRjOWYzYTBhMjJhYTA1ODljLi45 OTNlZDNkMGE1MjQ5NGViZWZlZGM0Njg4OWE2Mjg2ZjAxNTVlYjJiIDEwMDY0NAotLS0gYS9MYXlv dXRUZXN0cy9odHRwL3Rlc3RzL25hdmlnYXRpb24vcmVzb3VyY2VzL2dyYW5kY2hpbGQtd2l0aC1h bmNob3IuaHRtbAorKysgYi9MYXlvdXRUZXN0cy9odHRwL3Rlc3RzL25hdmlnYXRpb24vcmVzb3Vy Y2VzL2dyYW5kY2hpbGQtd2l0aC1hbmNob3IuaHRtbApAQCAtNSw1ICs1LDEwIEBACiAgIDwhLS0g U3BhY2VyIHRvIG1ha2Ugc3VyZSB0aGF0IHRoZSBuYW1lZCBhbmNob3IgYmVsb3cgcmVxdWlyZXMg c2Nyb2xsaW5nIC0tPgogPC9kaXY+CiA8YSBuYW1lPSJhbmNob3IxIj5UaGlzIGlzIGFuIGFuY2hv ciBwb2ludCBuYW1lZCAiYW5jaG9yMSI8L2E+LgorPHNjcmlwdD4KKy8vIFRlbXBvcmFyeSB3b3Jr YXJvdW5kIGZvciBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTQ4Njkw LgorLy8gTWFrZSBzdXJlIGxheW91dCBpcyBkb25lIGJlZm9yZSB3ZSBzY3JvbGwgdG8gdGhlIGFu Y2hvci4KK2RvY3VtZW50LmJvZHkub2Zmc2V0SGVpZ2h0OworPC9zY3JpcHQ+CiA8L2JvZHk+CiA8 L2h0bWw+Cg==