|Summary:||Page Scale affects window.scrollTo|
|Product:||WebKit||Reporter:||John Knottenbelt <jknotten>|
|Component:||New Bugs||Assignee:||John Knottenbelt <jknotten>|
|Severity:||Normal||CC:||aroben, bdakin, fsamuel, gns, gustavo.noronha, hclam, pnormand, simon.fraser, webkit.review.bot, xan.lopez|
|Version:||528+ (Nightly build)|
|Bug Depends on:|
Description John Knottenbelt 2011-09-14 08:25:32 PDT
Page Scale affects window.scrollTo
Comment 2 John Knottenbelt 2011-09-14 08:38:26 PDT
In this layout test there is a 3x2 grid of 100x100 coloured divs. The test case gives an expected behaviour of scrolling to page coordinates (100,100) - I expect to see just the bottom right two divs. However, when I run this layout test on DumpRenderTree, the actual visible output is a scroll to page coordinates (50, 50) -- half the top row and half the left column. Please let me know if my expected output (as in the patch) is incorrect. Thanks!
Comment 3 Simon Fraser (smfr) 2011-09-14 09:05:52 PDT
Comment on attachment 107334 [details] Patch The image result needs to go into a platform-specific directory, because it shows scrollbars. Please avoid red in successful pixel results.
Comment 5 Simon Fraser (smfr) 2011-09-19 10:59:42 PDT
Comment on attachment 107870 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=107870&action=review > LayoutTests/fast/events/scale-and-scroll.html:20 > + var scaleOffset = 0; Would be nice to test with a non-zero offset. > Source/WebCore/page/DOMWindow.cpp:1146 > + return static_cast<int>(view->scrollX() / m_frame->pageZoomFactor() / m_frame->pageScaleFactor()); I'd slightly prefer scrollX / (pagezoomfactor * pagescalefactor)
Comment 7 Hin-Chung Lam 2011-09-20 08:24:32 PDT
Actually the last patch didn't solve all the problem, namely iframes. I've corrected the root problem and added some more test cases.
Comment 8 WebKit Review Bot 2011-09-20 09:44:08 PDT
Comment on attachment 108004 [details] Patch Attachment 108004 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/9767300
Comment 9 Simon Fraser (smfr) 2011-09-20 10:09:38 PDT
Comment on attachment 108004 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=108004&action=review > Source/WebCore/ChangeLog:13 > + 2. Frame::frameScaleFactor() returns the scale factor of this frame with > + respect to the container. So for the main frame it'll return the scale > + factor of the page, inner frames will return 1.0 I wonder about behavioral changes as result of this. For example, was adjustForZoom() only ever called with the main frame's document before, or was there a bug? > Source/WebCore/page/FrameView.cpp:530 > + bool overrideHidden = m_frame->page() && m_frame->page()->mainFrame() == m_frame && m_frame->frameScaleFactor() > 1; Does this still require the m_frame->page()->mainFrame() == m_frame check?
Comment 10 Collabora GTK+ EWS bot 2011-09-20 15:42:03 PDT
Comment on attachment 108004 [details] Patch Attachment 108004 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/9764462
Comment 11 Hin-Chung Lam 2011-09-21 04:00:56 PDT
Committed r95608: <http://trac.webkit.org/changeset/95608>
Comment 12 Philippe Normand 2011-09-21 04:30:34 PDT
Can I know why this landed meanwhile it failed to pass EWS builds on 2 ports?
Comment 13 Hin-Chung Lam 2011-09-21 04:34:25 PDT
Whoops.. Sorry! I overlooked.. I'll fix those build failures now.
Comment 14 Philippe Normand 2011-09-21 04:43:44 PDT
I rolled it out before I saw your comment, sorry! I can help with GTK failures if needed. Please don't forget EWS next time, it's a great tool and it's sad to not take full advantage of it.
Comment 15 Hin-Chung Lam 2011-09-21 04:45:00 PDT
(In reply to comment #14) > I rolled it out before I saw your comment, sorry! I can help with GTK failures if needed. Please don't forget EWS next time, it's a great tool and it's sad to not take full advantage of it. Thanks for acting quick! Will be sure to use it next time, I'm fixing the patch now.
Comment 16 Hin-Chung Lam 2011-09-22 04:21:36 PDT
Created attachment 108310 [details] Patch for landing
Comment 17 WebKit Review Bot 2011-09-22 06:30:08 PDT
Comment on attachment 108310 [details] Patch for landing Clearing flags on attachment: 108310 Committed r95715: <http://trac.webkit.org/changeset/95715>
Comment 18 WebKit Review Bot 2011-09-22 06:30:15 PDT
All reviewed patches have been landed. Closing bug.