RESOLVED FIXED 41259
Interacting with a <select> element within a transformed and clipped container scrolls the container 
https://bugs.webkit.org/show_bug.cgi?id=41259
Summary Interacting with a <select> element within a transformed and clipped containe...
Antoine Quint
Reported 2010-06-27 06:01:14 PDT
Created attachment 59853 [details] Testcase See the attached testcase. Click on the <select> element, and notice how its position changes when the choices appear and does not move back to the original position once the choice is made. This reproduces both on desktop and on iOS 4.0 GM.
Attachments
Testcase (2.92 KB, text/html)
2010-06-27 06:01 PDT, Antoine Quint 		no flags
Details
patch and testcase (29.98 KB, patch)
2010-07-09 18:17 PDT, Dean Jackson 		simon.fraser: review-
DetailsFormatted DiffDiff
Patch and testcase (30.33 KB, patch)
2010-07-16 16:10 PDT, Dean Jackson 		simon.fraser: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Antoine Quint
Comment 1 2010-06-27 06:03:17 PDT
Removing the "overflow: hidden" style on the "container" element does not reproduce the issue.
Antoine Quint
Comment 2 2010-06-27 06:26:07 PDT
rdar://problem/8135236
mitz
Comment 3 2010-06-27 16:52:15 PDT
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.
Simon Fraser (smfr)
Comment 4 2010-06-28 09:52:51 PDT
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(); }
Dean Jackson
Comment 5 2010-07-02 13:09:10 PDT
Adding true as a parameter to absoluteBoundingBoxRect() did not fix this.
Simon Fraser (smfr)
Comment 6 2010-07-02 13:56:06 PDT
Are you sure?
Dean Jackson
Comment 7 2010-07-02 15:50:16 PDT
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.
Dean Jackson
Comment 8 2010-07-02 16:43:47 PDT
- 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.
Simon Fraser (smfr)
Comment 9 2010-07-02 16:48:58 PDT
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).
Dean Jackson
Comment 10 2010-07-09 18:17:10 PDT
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.
Dean Jackson
Comment 11 2010-07-09 19:34:14 PDT
simon suggests making the test be text output rather than render tree dump. this is fine with me
Simon Fraser (smfr)
Comment 12 2010-07-16 12:12:18 PDT
Comment on attachment 61132 [details] patch and testcase r- so that Dean can fix the tests.
Dean Jackson
Comment 13 2010-07-16 16:10:03 PDT
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.
Dean Jackson
Comment 14 2010-07-18 15:01:48 PDT
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.
Dean Jackson
Comment 15 2010-07-18 15:26:34 PDT
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.
Note You need to log in before you can comment on or make changes to this bug.