Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2455143003:
Fix mouse wheel events processing. (Closed)
DescriptionFix mouse wheel events processing.
Before: The mouse wheel event was processed twice in Aura.
this was broken in https://codereview.chromium.org/11592011/patch/1/66 in fix for BUG=163618
After: The mouse wheel event is processed once.
Also fixed EventGenerator::MoveMouseWheel under MacViews.
Committed: https://crrev.com/deff762c179bd5ebc206aee80418e91e9321bb4f
Cr-Commit-Position: refs/heads/master@{#428688}
Patch Set 1 #Patch Set 2 : fix missprint #
Total comments: 8
Patch Set 3 : Fix review issues. #
Messages
Total messages: 20 (13 generated)
Description was changed from ========== Fix mouse wheel events processing. Before: The mouse wheel event was double precessed in Aura. this was broken in https://codereview.chromium.org/11592011/patch/1/66 After: The mouse wheel event is processed once. Also fixed EventGenerator::MoveMouseWheel under MacViews. ========== to ========== Fix mouse wheel events processing. Before: The mouse wheel event was double precessed in Aura. this was broken in https://codereview.chromium.org/11592011/patch/1/66 After: The mouse wheel event is processed once. Also fixed EventGenerator::MoveMouseWheel under MacViews. ==========
art-snake@yandex-team.ru changed reviewers: + sadrul@chromium.org, tapted@chromium.org
Description was changed from ========== Fix mouse wheel events processing. Before: The mouse wheel event was double precessed in Aura. this was broken in https://codereview.chromium.org/11592011/patch/1/66 After: The mouse wheel event is processed once. Also fixed EventGenerator::MoveMouseWheel under MacViews. ========== to ========== Fix mouse wheel events processing. Before: The mouse wheel event was double processed in Aura. this was broken in https://codereview.chromium.org/11592011/patch/1/66 After: The mouse wheel event is processed once. Also fixed EventGenerator::MoveMouseWheel under MacViews. ==========
Description was changed from ========== Fix mouse wheel events processing. Before: The mouse wheel event was double processed in Aura. this was broken in https://codereview.chromium.org/11592011/patch/1/66 After: The mouse wheel event is processed once. Also fixed EventGenerator::MoveMouseWheel under MacViews. ========== to ========== Fix mouse wheel events processing. Before: The mouse wheel event was processed twice in Aura. this was broken in https://codereview.chromium.org/11592011/patch/1/66 After: The mouse wheel event is processed once. Also fixed EventGenerator::MoveMouseWheel under MacViews. ==========
lgtm. Please wait for tapted@'s review for mac change. https://codereview.chromium.org/2455143003/diff/20001/ui/views/widget/widget_... File ui/views/widget/widget_unittest.cc (right): https://codereview.chromium.org/2455143003/diff/20001/ui/views/widget/widget_... ui/views/widget/widget_unittest.cc:3631: Widget* widget = CreateTopLevelPlatformWidget(); Like the tests above, use WidgetAutoclosePtr instead?
The CQ bit was checked by art-snake@yandex-team.ru 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 with nits and sadrul's comment addressed - thanks! I'd also add BUG=163618 (Or make a new bug if there's a perceivable behaviour change) https://codereview.chromium.org/2455143003/diff/20001/ui/views/test/event_gen... File ui/views/test/event_generator_delegate_mac.mm (right): https://codereview.chromium.org/2455143003/diff/20001/ui/views/test/event_gen... ui/views/test/event_generator_delegate_mac.mm:222: DCHECK(mouse_event->type() == ui::ET_MOUSEWHEEL); nit: DCHECK_EQ https://codereview.chromium.org/2455143003/diff/20001/ui/views/test/event_gen... ui/views/test/event_generator_delegate_mac.mm:224: static_cast<ui::MouseWheelEvent*>(mouse_event); mouse_event->AsMouseWheelEvent() https://codereview.chromium.org/2455143003/diff/20001/ui/views/widget/widget_... File ui/views/widget/widget_unittest.cc (right): https://codereview.chromium.org/2455143003/diff/20001/ui/views/widget/widget_... ui/views/widget/widget_unittest.cc:3630: TEST_F(WidgetTest, MouseWheelEvent) { nit: tests should have a comment describing their purpose e.g. "// Test dispatch of ui::ET_MOUSEWHEEL."
Description was changed from ========== Fix mouse wheel events processing. Before: The mouse wheel event was processed twice in Aura. this was broken in https://codereview.chromium.org/11592011/patch/1/66 After: The mouse wheel event is processed once. Also fixed EventGenerator::MoveMouseWheel under MacViews. ========== to ========== Fix mouse wheel events processing. Before: The mouse wheel event was processed twice in Aura. this was broken in https://codereview.chromium.org/11592011/patch/1/66 in fix for BUG=163618 After: The mouse wheel event is processed once. Also fixed EventGenerator::MoveMouseWheel under MacViews. ==========
https://codereview.chromium.org/2455143003/diff/20001/ui/views/test/event_gen... File ui/views/test/event_generator_delegate_mac.mm (right): https://codereview.chromium.org/2455143003/diff/20001/ui/views/test/event_gen... ui/views/test/event_generator_delegate_mac.mm:222: DCHECK(mouse_event->type() == ui::ET_MOUSEWHEEL); On 2016/10/30 23:54:46, tapted wrote: > nit: DCHECK_EQ Done. https://codereview.chromium.org/2455143003/diff/20001/ui/views/test/event_gen... ui/views/test/event_generator_delegate_mac.mm:224: static_cast<ui::MouseWheelEvent*>(mouse_event); On 2016/10/30 23:54:46, tapted wrote: > mouse_event->AsMouseWheelEvent() Done. https://codereview.chromium.org/2455143003/diff/20001/ui/views/widget/widget_... File ui/views/widget/widget_unittest.cc (right): https://codereview.chromium.org/2455143003/diff/20001/ui/views/widget/widget_... ui/views/widget/widget_unittest.cc:3630: TEST_F(WidgetTest, MouseWheelEvent) { On 2016/10/30 23:54:46, tapted wrote: > nit: tests should have a comment describing their purpose e.g. "// Test dispatch > of ui::ET_MOUSEWHEEL." Done. https://codereview.chromium.org/2455143003/diff/20001/ui/views/widget/widget_... ui/views/widget/widget_unittest.cc:3631: Widget* widget = CreateTopLevelPlatformWidget(); On 2016/10/29 05:52:54, sadrul wrote: > Like the tests above, use WidgetAutoclosePtr instead? Done.
The CQ bit was checked by art-snake@yandex-team.ru
The patchset sent to the CQ was uploaded after l-g-t-m from tapted@chromium.org, sadrul@chromium.org Link to the patchset: https://codereview.chromium.org/2455143003/#ps40001 (title: "Fix review issues.")
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 ========== Fix mouse wheel events processing. Before: The mouse wheel event was processed twice in Aura. this was broken in https://codereview.chromium.org/11592011/patch/1/66 in fix for BUG=163618 After: The mouse wheel event is processed once. Also fixed EventGenerator::MoveMouseWheel under MacViews. ========== to ========== Fix mouse wheel events processing. Before: The mouse wheel event was processed twice in Aura. this was broken in https://codereview.chromium.org/11592011/patch/1/66 in fix for BUG=163618 After: The mouse wheel event is processed once. Also fixed EventGenerator::MoveMouseWheel under MacViews. ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Fix mouse wheel events processing. Before: The mouse wheel event was processed twice in Aura. this was broken in https://codereview.chromium.org/11592011/patch/1/66 in fix for BUG=163618 After: The mouse wheel event is processed once. Also fixed EventGenerator::MoveMouseWheel under MacViews. ========== to ========== Fix mouse wheel events processing. Before: The mouse wheel event was processed twice in Aura. this was broken in https://codereview.chromium.org/11592011/patch/1/66 in fix for BUG=163618 After: The mouse wheel event is processed once. Also fixed EventGenerator::MoveMouseWheel under MacViews. Committed: https://crrev.com/deff762c179bd5ebc206aee80418e91e9321bb4f Cr-Commit-Position: refs/heads/master@{#428688} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/deff762c179bd5ebc206aee80418e91e9321bb4f Cr-Commit-Position: refs/heads/master@{#428688} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
