Bug 41259

Removing the "overflow: hidden" style on the "container" element does not reproduce the issue.

You can see similar behavior when focusing the review flag popup in the bugs.webkit.org review form. I thought I’d filed a bug about it but I couldn’t fine one when I searched now.

The bug is the use of getRect in Element::updateFocusAppearance(): renderer()->enclosingLayer()->scrollRectToVisible(getRect()); IntRect Node::getRect() const { // FIXME: broken with transforms if (renderer()) return renderer()->absoluteBoundingBoxRect(); return IntRect(); }

Adding true as a parameter to absoluteBoundingBoxRect() did not fix this.

Are you sure?

The testcase still behaved the same way. run-webkit-tests output didn't change either. The reason is that the code doesn't go through Node::getRect(). It hits ContainerNode::getRect which has a very different implementation.

- point = o->localToAbsolute(); + point = o->localToAbsolute(FloatPoint(), false, true); in ContainerNode's get TopLeft and BottomRight work, and I don't see any regressions. Preparing a patch with a real test.

I think that will do the wrong thing if you hit either of the point.move() calls (since that will move the point post-transform, rather than pre-transform).

Created attachment 61132 [details] patch and testcase Patch and testcase. The ContainerNode::getRect call (and the two methods changed here) are quite concerning. Many of the comments indicate that its behaviour has been lost in time. Anyway, this patch fixes the raised bug, hopefully makes all of this code transform aware, and has a test case to make sure nothing has broken. The test case exercises many of the code paths in the ContainerNode methods.

simon suggests making the test be text output rather than render tree dump. this is fine with me

Comment on attachment 61132 [details] patch and testcase r- so that Dean can fix the tests.

Created attachment 61857 [details] Patch and testcase New testcase that is text-based using scrollTop rather than rendertree-based. This also tests the untransformed case to show that things have not changed with the addition of transforms. The test calls focus on the form elements which should bring them into view. This was the origin of the bug, that popups would appear at a different location from the click and menu.

Committing to http://svn.webkit.org/repository/webkit/trunk ... M LayoutTests/ChangeLog A LayoutTests/fast/transforms/scrollIntoView-transformed-expected.txt A LayoutTests/fast/transforms/scrollIntoView-transformed.html M WebCore/ChangeLog M WebCore/WebCore.xcodeproj/project.pbxproj M WebCore/dom/ContainerNode.cpp M WebCore/dom/Node.cpp Committed r63633 I removed the xcode barf-up in the next commit.

Committing to http://svn.webkit.org/repository/webkit/trunk ... M LayoutTests/ChangeLog M LayoutTests/fast/transforms/scrollIntoView-transformed-expected.txt M LayoutTests/fast/transforms/scrollIntoView-transformed.html Committed r63635 Adding a test that hopefully won't fail on non-mac platforms.