WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
68081
Page Scale affects window.scrollTo
https://bugs.webkit.org/show_bug.cgi?id=68081
Summary
Page Scale affects window.scrollTo
John Knottenbelt
Reported
2011-09-14 08:25:32 PDT
Page Scale affects window.scrollTo
Attachments
Patch
(5.03 KB, patch)
2011-09-14 08:26 PDT
,
John Knottenbelt
no flags
Details
Formatted Diff
Diff
Patch
(7.08 KB, patch)
2011-09-19 09:14 PDT
,
Hin-Chung Lam
no flags
Details
Formatted Diff
Diff
Patch
(26.66 KB, patch)
2011-09-20 08:23 PDT
,
Hin-Chung Lam
no flags
Details
Formatted Diff
Diff
Patch for landing
(30.21 KB, patch)
2011-09-22 04:21 PDT
,
Hin-Chung Lam
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
John Knottenbelt
Comment 1
2011-09-14 08:26:13 PDT
Created
attachment 107334
[details]
Patch
John Knottenbelt
Comment 2
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!
Simon Fraser (smfr)
Comment 3
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.
Hin-Chung Lam
Comment 4
2011-09-19 09:14:52 PDT
Created
attachment 107870
[details]
Patch
Simon Fraser (smfr)
Comment 5
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)
Hin-Chung Lam
Comment 6
2011-09-20 08:23:47 PDT
Created
attachment 108004
[details]
Patch
Hin-Chung Lam
Comment 7
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.
WebKit Review Bot
Comment 8
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
Simon Fraser (smfr)
Comment 9
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?
Collabora GTK+ EWS bot
Comment 10
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
Hin-Chung Lam
Comment 11
2011-09-21 04:00:56 PDT
Committed
r95608
: <
http://trac.webkit.org/changeset/95608
>
Philippe Normand
Comment 12
2011-09-21 04:30:34 PDT
Can I know why this landed meanwhile it failed to pass EWS builds on 2 ports?
Hin-Chung Lam
Comment 13
2011-09-21 04:34:25 PDT
Whoops.. Sorry! I overlooked.. I'll fix those build failures now.
Philippe Normand
Comment 14
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.
Hin-Chung Lam
Comment 15
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.
Hin-Chung Lam
Comment 16
2011-09-22 04:21:36 PDT
Created
attachment 108310
[details]
Patch for landing
WebKit Review Bot
Comment 17
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
>
WebKit Review Bot
Comment 18
2011-09-22 06:30:15 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug