Bug 65800

Summary: SVG panning y-axis is flipped on Windows and WebKit2/Mac
Product: WebKit Reporter: Tim Horton <thorton>
Component: SVGAssignee: Tim Horton <thorton>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, dglazkov, simon.fraser, webkit.review.bot, wjmaclean, zimmermann
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
webkit.review.bot: commit-queue-
Chromium-linux pixel results for patch.
none
with cr-linux results (?) darin: review+

Tim Horton
Reported Saturday, August 6, 2011 1:27:30 AM UTC
1. Open a large SVG document, for example, http://upload.wikimedia.org/wikipedia/commons/7/74/Timeline_of_web_browsers.svg 2. Pan up and down with shift-drag. Expected result: Image pans with mouse. Actual result: Image pans in opposite direction of mouse in y-axis. I have a patch which I will attach shortly. <rdar://problem/9908012> for WK2 <rdar://problem/5700027> for Windows
Attachments
Patch (9.56 KB, patch)
2011-08-08 10:24 PDT, Tim Horton
webkit.review.bot: commit-queue-
Chromium-linux pixel results for patch. (2.80 KB, image/png)
2011-08-16 11:42 PDT, W. James MacLean
no flags
with cr-linux results (?) (10.48 KB, patch)
2011-08-29 16:49 PDT, Tim Horton
darin: review+
Tim Horton
Comment 1 Monday, August 8, 2011 6:24:16 PM UTC
Created attachment 103262 [details] Patch I'm going to test it in Windows first and see if it fixes the problem there before committing, and I'll add that Radar # too if it does.
WebKit Review Bot
Comment 2 Monday, August 8, 2011 8:23:49 PM UTC
Comment on attachment 103262 [details] Patch Attachment 103262 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9328956 New failing tests: svg/custom/pan-direction.svg
W. James MacLean
Comment 3 Tuesday, August 16, 2011 7:26:27 PM UTC
*** Bug 66315 has been marked as a duplicate of this bug. ***
W. James MacLean
Comment 4 Tuesday, August 16, 2011 7:42:22 PM UTC
Created attachment 104074 [details] Chromium-linux pixel results for patch.
W. James MacLean
Comment 5 Friday, August 26, 2011 6:29:49 PM UTC
(In reply to comment #1) > Created an attachment (id=103262) [details] > Patch > > I'm going to test it in Windows first and see if it fixes the problem there before committing, and I'll add that Radar # too if it does. Any idea when this patch will land? There are outstanding bugs filed against it that would be nice to close. Let me know if I can help.
Tim Horton
Comment 6 Saturday, August 27, 2011 9:49:10 AM UTC
(In reply to comment #5) > (In reply to comment #1) > > Created an attachment (id=103262) [details] [details] > > Patch > > > > I'm going to test it in Windows first and see if it fixes the problem there before committing, and I'll add that Radar # too if it does. > > Any idea when this patch will land? There are outstanding bugs filed against it that would be nice to close. Let me know if I can help. I'll try to get it through as soon as I can this week; I have a backlog of patches, so I should spend some time getting them landed.
Tim Horton
Comment 7 Monday, August 29, 2011 11:36:15 PM UTC
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #1) > > > Created an attachment (id=103262) [details] [details] [details] > > > Patch > > > > > > I'm going to test it in Windows first and see if it fixes the problem there before committing, and I'll add that Radar # too if it does. > > > > Any idea when this patch will land? There are outstanding bugs filed against it that would be nice to close. Let me know if I can help. > > I'll try to get it through as soon as I can this week; I have a backlog of patches, so I should spend some time getting them landed. Oddly, the image you attached is pixel-identical to the one in the test; I wonder why the bot says cr-linux doesn't match my result.
Tim Horton
Comment 8 Tuesday, August 30, 2011 12:49:36 AM UTC
Created attachment 105543 [details] with cr-linux results (?)
Darin Adler
Comment 9 Tuesday, August 30, 2011 2:07:14 AM UTC
Comment on attachment 105543 [details] with cr-linux results (?) View in context: https://bugs.webkit.org/attachment.cgi?id=105543&action=review > Source/WebCore/page/EventHandler.cpp:477 > + FrameView* view = m_frame->view(); > + static_cast<SVGDocument*>(m_frame->document())->startPan(view->windowToContents(event.event().pos())); Why the local variable? I think it would read better without it. > Source/WebCore/page/EventHandler.cpp:1566 > + FrameView* view = m_frame->view(); > + static_cast<SVGDocument*>(m_frame->document())->updatePan(view->windowToContents(m_currentMousePosition)); Ditto. > Source/WebCore/page/EventHandler.cpp:1688 > + FrameView* view = m_frame->view(); > + static_cast<SVGDocument*>(m_frame->document())->updatePan(view->windowToContents(m_currentMousePosition)); Ditto.
Tim Horton
Comment 10 Tuesday, August 30, 2011 11:02:44 PM UTC
Landed in 94114.
W. James MacLean
Comment 11 Wednesday, August 31, 2011 7:44:59 PM UTC
(In reply to comment #10) > Landed in 94114. Thanks, looks good!
Simon Fraser (smfr)
Comment 12 Wednesday, August 31, 2011 10:08:41 PM UTC
Caused bug 67318?
Alexey Proskuryakov
Comment 13 Wednesday, August 31, 2011 10:33:24 PM UTC
Seems unlikely.
Note You need to log in before you can comment on or make changes to this bug.