Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2486643002:
Mac: Offset test scroll events when attaching them to an NSWindow. (Closed)
|
Created:
4 years, 1 month ago by tapted Modified:
4 years, 1 month ago Reviewers:
Avi (use Gerrit) CC:
chromium-reviews, mac-reviews_chromium.org, tdresser+watch_chromium.org, chrome-apps-syd-reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMac: Offset test scroll events when attaching them to an NSWindow.
This fixes WidgetTest.MouseWheelEvent on Mac. Currently it's affected by
the screen height: It passes on the bots because the window is large
enough to fill most of the (virtual) screen. However, on larger screens,
it fails.
It hasn't been a problem in the past because tests using
cocoa_test_event_utils::TestScrollEvent() typically don't care about the
x/y location of the event, but WidgetTest.MouseWheelEvent does.
BUG=662763
Committed: https://crrev.com/233eb52fb2c8eeeec2eb27a9fdb08e66836974cc
Cr-Commit-Position: refs/heads/master@{#430751}
Patch Set 1 #Patch Set 2 : fix diff #Patch Set 3 : not needed #
Total comments: 3
Dependent Patchsets: Messages
Total messages: 24 (17 generated)
The CQ bit was checked by tapted@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by tapted@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Mac: Offset test scroll events when attaching them to an NSWindow. BUG=662763 ========== to ========== Mac: Offset test scroll events when attaching them to an NSWindow. This fixes WidgetTest.MouseWheelEvent on Mac. Currently it's affected by the screen height: It passes on the bots because the window is large enough to fill most of the (virtual) screen. It hasn't been a problem in the past because tests using cocoa_test_event_utils::TestScrollEvent() typically don't care about the x/y location of the event, but WidgetTest.MouseWheelEvent does. BUG=662763 ==========
tapted@chromium.org changed reviewers: + karandeepb@chromium.org
Description was changed from ========== Mac: Offset test scroll events when attaching them to an NSWindow. This fixes WidgetTest.MouseWheelEvent on Mac. Currently it's affected by the screen height: It passes on the bots because the window is large enough to fill most of the (virtual) screen. It hasn't been a problem in the past because tests using cocoa_test_event_utils::TestScrollEvent() typically don't care about the x/y location of the event, but WidgetTest.MouseWheelEvent does. BUG=662763 ========== to ========== Mac: Offset test scroll events when attaching them to an NSWindow. This fixes WidgetTest.MouseWheelEvent on Mac. Currently it's affected by the screen height: It passes on the bots because the window is large enough to fill most of the (virtual) screen. However, on larger screens, it fails. It hasn't been a problem in the past because tests using cocoa_test_event_utils::TestScrollEvent() typically don't care about the x/y location of the event, but WidgetTest.MouseWheelEvent does. BUG=662763 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
tapted@chromium.org changed reviewers: + avi@chromium.org - karandeepb@chromium.org
Hi avi, please take a look. Thanks!
The CQ bit was checked by tapted@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2486643002/diff/40001/ui/events/test/cocoa_te... File ui/events/test/cocoa_test_event_utils.mm (right): https://codereview.chromium.org/2486643002/diff/40001/ui/events/test/cocoa_te... ui/events/test/cocoa_test_event_utils.mm:37: // origin, but in different directions for x/y. o_O Is there a bug filed? This is surprising. (Though not really.)
https://codereview.chromium.org/2486643002/diff/40001/ui/events/test/cocoa_te... File ui/events/test/cocoa_test_event_utils.mm (right): https://codereview.chromium.org/2486643002/diff/40001/ui/events/test/cocoa_te... ui/events/test/cocoa_test_event_utils.mm:37: // origin, but in different directions for x/y. On 2016/11/08 16:20:47, Avi wrote: > o_O > > Is there a bug filed? This is surprising. (Though not really.) No bug.. It's annoying, but I think it's reasonable for things to be this way. It would be "nice" if NSEvent provided ways to make test events specifically for trackpad scrolling, but it's often device-specific, and Apple's opinion is probably that only NSScrollView should ever care about them and NSScrollView will do the Right Thing for you. Instead, all they give is +[NSEvent eventWithCGEvent:] - there's no way to pass in an NSWindow with this initializer, so it's reasonable that NSEvent caters for part of the coordinate conversion, but not all of it. It would also be nice if NSEvent had some mutators - its API is immutable, but AppKit has access to the internals and tinkers with things during event dispatch. And while I'm at it, it would be nice if Mac didn't use different coordinate systems for CG events and NS events :p I don't see NSEvent becoming mutable any time soon, so I think all I can really do is file a rdar asking for better methods to create test NSEvents for scrolling.
The CQ bit was checked by tapted@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Mac: Offset test scroll events when attaching them to an NSWindow. This fixes WidgetTest.MouseWheelEvent on Mac. Currently it's affected by the screen height: It passes on the bots because the window is large enough to fill most of the (virtual) screen. However, on larger screens, it fails. It hasn't been a problem in the past because tests using cocoa_test_event_utils::TestScrollEvent() typically don't care about the x/y location of the event, but WidgetTest.MouseWheelEvent does. BUG=662763 ========== to ========== Mac: Offset test scroll events when attaching them to an NSWindow. This fixes WidgetTest.MouseWheelEvent on Mac. Currently it's affected by the screen height: It passes on the bots because the window is large enough to fill most of the (virtual) screen. However, on larger screens, it fails. It hasn't been a problem in the past because tests using cocoa_test_event_utils::TestScrollEvent() typically don't care about the x/y location of the event, but WidgetTest.MouseWheelEvent does. BUG=662763 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Mac: Offset test scroll events when attaching them to an NSWindow. This fixes WidgetTest.MouseWheelEvent on Mac. Currently it's affected by the screen height: It passes on the bots because the window is large enough to fill most of the (virtual) screen. However, on larger screens, it fails. It hasn't been a problem in the past because tests using cocoa_test_event_utils::TestScrollEvent() typically don't care about the x/y location of the event, but WidgetTest.MouseWheelEvent does. BUG=662763 ========== to ========== Mac: Offset test scroll events when attaching them to an NSWindow. This fixes WidgetTest.MouseWheelEvent on Mac. Currently it's affected by the screen height: It passes on the bots because the window is large enough to fill most of the (virtual) screen. However, on larger screens, it fails. It hasn't been a problem in the past because tests using cocoa_test_event_utils::TestScrollEvent() typically don't care about the x/y location of the event, but WidgetTest.MouseWheelEvent does. BUG=662763 Committed: https://crrev.com/233eb52fb2c8eeeec2eb27a9fdb08e66836974cc Cr-Commit-Position: refs/heads/master@{#430751} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/233eb52fb2c8eeeec2eb27a9fdb08e66836974cc Cr-Commit-Position: refs/heads/master@{#430751}
Message was sent while issue was closed.
https://codereview.chromium.org/2486643002/diff/40001/ui/events/test/cocoa_te... File ui/events/test/cocoa_test_event_utils.mm (right): https://codereview.chromium.org/2486643002/diff/40001/ui/events/test/cocoa_te... ui/events/test/cocoa_test_event_utils.mm:37: // origin, but in different directions for x/y. On 2016/11/08 23:05:52, tapted wrote: > On 2016/11/08 16:20:47, Avi wrote: > > o_O > > > > Is there a bug filed? This is surprising. (Though not really.) > > No bug.. It's annoying, but I think it's reasonable for things to be this way. > It would be "nice" if NSEvent provided ways to make test events specifically for > trackpad scrolling, but it's often device-specific, and Apple's opinion is > probably that only NSScrollView should ever care about them and NSScrollView > will do the Right Thing for you. > > Instead, all they give is +[NSEvent eventWithCGEvent:] - there's no way to pass > in an NSWindow with this initializer, so it's reasonable that NSEvent caters for > part of the coordinate conversion, but not all of it. > > It would also be nice if NSEvent had some mutators - its API is immutable, but > AppKit has access to the internals and tinkers with things during event > dispatch. > > And while I'm at it, it would be nice if Mac didn't use different coordinate > systems for CG events and NS events :p > > I don't see NSEvent becoming mutable any time soon, so I think all I can really > do is file a rdar asking for better methods to create test NSEvents for > scrolling. Filed http://crbug.com/663553 that collects my current research around cocoa_test_event_utils:: and the things I dream of doing to it one day. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
