Bug 89405 - West-panning cursor does not match resize cursor, unlike all other directions
Summary: West-panning cursor does not match resize cursor, unlike all other directions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Avi Drissman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-18 17:31 PDT by Avi Drissman
Modified: 2013-01-16 09:57 PST (History)
3 users (show)

See Also:


Attachments
Patch (1.45 KB, patch)
2012-06-18 17:36 PDT, Avi Drissman
darin: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
updated version (1.39 KB, patch)
2013-01-16 08:11 PST, Avi Drissman
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Avi Drissman 2012-06-18 17:31:01 PDT
In WebCore/platform/mac/CursorMac.mm, the panning and resize cursors all match, except for west. With the west panning cursor, it's mapped to EastWestResize.
Comment 1 Avi Drissman 2012-06-18 17:36:24 PDT
Created attachment 148212 [details]
Patch
Comment 2 Alexey Proskuryakov 2012-06-19 11:43:38 PDT
Do you have a test case showing this in action?
Comment 3 Avi Drissman 2012-06-19 11:56:41 PDT
There are two ways to get the west panning cursor.

One is to explicitly call Cursor::westPanningCursor, which is only called from EventHandler::updatePanScrollState. Since PAN_SCROLLING isn't enabled on the Mac, this case is never seen.

The other way to get that cursor is Cursor::fromType(Cursor::WestPanning), which is only called from WebKit2/Shared/WebCoreArgumentCoders.cpp, ArgumentCoder<Cursor>::decode. I'm not sure how to trace that back.
Comment 4 Avi Drissman 2012-07-17 11:53:05 PDT
I don't see what the issue is here. Panning cursors aren't usually compiled into the Mac build, so if we have them in CursorMac at all, we might as well get them right.
Comment 5 WebKit Review Bot 2013-01-15 15:43:06 PST
Comment on attachment 148212 [details]
Patch

Rejecting attachment 148212 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
: /mnt/git/webkit-commit-queue

Parsed 2 diffs from patch file(s).
patching file Source/WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebCore/platform/mac/CursorMac.mm
Hunk #1 FAILED at 188.
Hunk #2 FAILED at 204.
2 out of 2 hunks FAILED -- saving rejects to file Source/WebCore/platform/mac/CursorMac.mm.rej

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force', '--reviewer', 'Darin Adler']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/15901149
Comment 6 Avi Drissman 2013-01-15 22:02:38 PST
Darin,

Thank you for the r+; I will upload a new version of this patch tomorrow.
Comment 7 Avi Drissman 2013-01-16 08:11:50 PST
Created attachment 182984 [details]
updated version
Comment 8 Alexey Proskuryakov 2013-01-16 09:47:19 PST
Comment on attachment 182984 [details]
updated version

This was already reviewed, so you could have updated ChangeLog with Darin's name, and only requested cq? flag.
Comment 9 WebKit Review Bot 2013-01-16 09:50:28 PST
Comment on attachment 182984 [details]
updated version

Clearing flags on attachment: 182984

Committed r139889: <http://trac.webkit.org/changeset/139889>
Comment 10 WebKit Review Bot 2013-01-16 09:50:32 PST
All reviewed patches have been landed.  Closing bug.
Comment 11 Avi Drissman 2013-01-16 09:57:18 PST
(In reply to comment #8)
> (From update of attachment 182984 [details])
> This was already reviewed, so you could have updated ChangeLog with Darin's name, and only requested cq? flag.

OK. I'm not that familiar with the WebKit review process, so the next time I will keep that in mind.