Bug 65800 - SVG panning y-axis is flipped on Windows and WebKit2/Mac
Summary: SVG panning y-axis is flipped on Windows and WebKit2/Mac
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords: InRadar
: 66315 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-08-05 17:27 PDT by Tim Horton
Modified: 2011-08-31 14:33 PDT (History)
6 users (show)

See Also:


Attachments
Patch (9.56 KB, patch)
2011-08-08 10:24 PDT, Tim Horton
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Chromium-linux pixel results for patch. (2.80 KB, image/png)
2011-08-16 11:42 PDT, W. James MacLean
no flags Details
with cr-linux results (?) (10.48 KB, patch)
2011-08-29 16:49 PDT, Tim Horton
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2011-08-05 17:27:30 PDT
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
Comment 1 Tim Horton 2011-08-08 10:24:16 PDT
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.
Comment 2 WebKit Review Bot 2011-08-08 12:23:49 PDT
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
Comment 3 W. James MacLean 2011-08-16 11:26:27 PDT
*** Bug 66315 has been marked as a duplicate of this bug. ***
Comment 4 W. James MacLean 2011-08-16 11:42:22 PDT
Created attachment 104074 [details]
Chromium-linux pixel results for patch.
Comment 5 W. James MacLean 2011-08-26 10:29:49 PDT
(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.
Comment 6 Tim Horton 2011-08-27 01:49:10 PDT
(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.
Comment 7 Tim Horton 2011-08-29 15:36:15 PDT
(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.
Comment 8 Tim Horton 2011-08-29 16:49:36 PDT
Created attachment 105543 [details]
with cr-linux results (?)
Comment 9 Darin Adler 2011-08-29 18:07:14 PDT
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.
Comment 10 Tim Horton 2011-08-30 15:02:44 PDT
Landed in 94114.
Comment 11 W. James MacLean 2011-08-31 11:44:59 PDT
(In reply to comment #10)
> Landed in 94114.

Thanks, looks good!
Comment 12 Simon Fraser (smfr) 2011-08-31 14:08:41 PDT
Caused bug 67318?
Comment 13 Alexey Proskuryakov 2011-08-31 14:33:24 PDT
Seems unlikely.