Issue metadata
Sign in to add a comment
|
Physical keyboard arrow key scrolling broken on Android |
||||||||||||||||||||||
Issue description"Commit 1cd6848f54 (http://crrev.com/2206053002 ) appears to have removed the ability to scroll webpages using arrow keys on Android. I believe the reason for this is WebViewImpl::keyEventDefault [1] only calls through to WebViewImpl::scrollViewWithKeyboard for arrow keys if the WebKeyboardEvent type is RawKeyDown [2]. Now that the type is KeyDown, keyEventDefault no longer handles the event and you can no longer scroll on webpages using the arrow keys. Note: Since this change didn't land in v53, keyboard scrolling still works there but in ChromeBeta (v54) keyboard scrolling doesn't work. [1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/web/WebViewImpl.cpp?rcl=0&l=1666 [2] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/web/WebViewImpl.cpp?dr=CSs&q=webviewimpl&sq=package:chromium&l=1703"
,
Oct 19 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5492e01d2bb022d2bdfab86250f5a4c543cea122 commit 5492e01d2bb022d2bdfab86250f5a4c543cea122 Author: aelias <aelias@chromium.org> Date: Wed Oct 19 21:28:03 2016 Add coalesced KeyDown handling for arrow key scrolling. Android switched to sending coalesced KeyDown instead of RawKeyDown/Char pair in http://crrev.com/2206053002. The arrow key handler in Source/web in particular did not account for this case, therefore arrow keys stopped scrolling on Android (although all other uses of arrow keys handled in Source/core still work). NOTRY=true BUG= 657158 Review-Url: https://chromiumcodereview.appspot.com/2434463003 Cr-Commit-Position: refs/heads/master@{#426289} [modify] https://crrev.com/5492e01d2bb022d2bdfab86250f5a4c543cea122/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp [modify] https://crrev.com/5492e01d2bb022d2bdfab86250f5a4c543cea122/third_party/WebKit/Source/web/WebViewImpl.cpp
,
Oct 19 2016
,
Oct 19 2016
[Automated comment] Request affecting a post-stable build (M54), manual review required.
,
Oct 19 2016
Your change meets the bar and is auto-approved for M55 (branch: 2883)
,
Oct 19 2016
[Automated comment] Request affecting a post-stable build (M54), manual review required.
,
Oct 19 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/59f0841b8ba5acde00049a4fe6c27bae0cd519fa commit 59f0841b8ba5acde00049a4fe6c27bae0cd519fa Author: Alexandre Elias <aelias@chromium.org> Date: Wed Oct 19 21:47:57 2016 Add coalesced KeyDown handling for arrow key scrolling. Android switched to sending coalesced KeyDown instead of RawKeyDown/Char pair in http://crrev.com/2206053002. The arrow key handler in Source/web in particular did not account for this case, therefore arrow keys stopped scrolling on Android (although all other uses of arrow keys handled in Source/core still work). NOTRY=true BUG= 657158 Review-Url: https://chromiumcodereview.appspot.com/2434463003 Cr-Commit-Position: refs/heads/master@{#426289} (cherry picked from commit 5492e01d2bb022d2bdfab86250f5a4c543cea122) Review URL: https://codereview.chromium.org/2434013003 . Cr-Commit-Position: refs/branch-heads/2883@{#196} Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768} [modify] https://crrev.com/59f0841b8ba5acde00049a4fe6c27bae0cd519fa/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp [modify] https://crrev.com/59f0841b8ba5acde00049a4fe6c27bae0cd519fa/third_party/WebKit/Source/web/WebViewImpl.cpp
,
Oct 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3f0b7ec4a4573fff9623546c348ee8fa9353d38c commit 3f0b7ec4a4573fff9623546c348ee8fa9353d38c Author: aelias <aelias@chromium.org> Date: Thu Oct 27 03:21:30 2016 Move arrow scrolling into KeyboardEventManager. This moves the default arrow/pageup/home/end handler (resulting in a smooth scroll animation) next to all the other keyboard default handlers. Notes: - The spacebar scrolling and Ctrl-C/A codepaths in WebViewImpl were dead code and have likely been for years. These keys were already handled by WebCore before reaching WebViewImpl::keyEventDefault. So I simply deleted them. - One calendar layout test was timing out on Mac, relating to how Alt-Down is also a shortcut for opening the calendar month flyout. I couldn't repro any problem manually, and it only happens in a racy close-flyout-then-reopen scenario which seems like an artifact of test structure more than an intended test case, so I just merged the two parts of the test. BUG= 657158 Review-Url: https://codereview.chromium.org/2430183002 Cr-Commit-Position: refs/heads/master@{#427925} [modify] https://crrev.com/3f0b7ec4a4573fff9623546c348ee8fa9353d38c/third_party/WebKit/LayoutTests/fast/forms/calendar-picker/calendar-picker-key-operations-expected.txt [modify] https://crrev.com/3f0b7ec4a4573fff9623546c348ee8fa9353d38c/third_party/WebKit/LayoutTests/fast/forms/calendar-picker/calendar-picker-key-operations.html [modify] https://crrev.com/3f0b7ec4a4573fff9623546c348ee8fa9353d38c/third_party/WebKit/Source/core/input/KeyboardEventManager.cpp [modify] https://crrev.com/3f0b7ec4a4573fff9623546c348ee8fa9353d38c/third_party/WebKit/Source/core/input/KeyboardEventManager.h [modify] https://crrev.com/3f0b7ec4a4573fff9623546c348ee8fa9353d38c/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp [modify] https://crrev.com/3f0b7ec4a4573fff9623546c348ee8fa9353d38c/third_party/WebKit/Source/web/WebFrameWidgetImpl.h [modify] https://crrev.com/3f0b7ec4a4573fff9623546c348ee8fa9353d38c/third_party/WebKit/Source/web/WebViewImpl.cpp [modify] https://crrev.com/3f0b7ec4a4573fff9623546c348ee8fa9353d38c/third_party/WebKit/Source/web/WebViewImpl.h [modify] https://crrev.com/3f0b7ec4a4573fff9623546c348ee8fa9353d38c/third_party/WebKit/Source/web/tests/WebViewTest.cpp
,
Oct 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/59f0841b8ba5acde00049a4fe6c27bae0cd519fa commit 59f0841b8ba5acde00049a4fe6c27bae0cd519fa Author: Alexandre Elias <aelias@chromium.org> Date: Wed Oct 19 21:47:57 2016 Add coalesced KeyDown handling for arrow key scrolling. Android switched to sending coalesced KeyDown instead of RawKeyDown/Char pair in http://crrev.com/2206053002. The arrow key handler in Source/web in particular did not account for this case, therefore arrow keys stopped scrolling on Android (although all other uses of arrow keys handled in Source/core still work). NOTRY=true BUG= 657158 Review-Url: https://chromiumcodereview.appspot.com/2434463003 Cr-Commit-Position: refs/heads/master@{#426289} (cherry picked from commit 5492e01d2bb022d2bdfab86250f5a4c543cea122) Review URL: https://codereview.chromium.org/2434013003 . Cr-Commit-Position: refs/branch-heads/2883@{#196} Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768} [modify] https://crrev.com/59f0841b8ba5acde00049a4fe6c27bae0cd519fa/third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp [modify] https://crrev.com/59f0841b8ba5acde00049a4fe6c27bae0cd519fa/third_party/WebKit/Source/web/WebViewImpl.cpp
,
Oct 28 2016
To be clear, this is *not* merged to 54 yet. bugdroid seems to be going ballistic with bogus merge-merged-2840 labels right now.
,
Oct 28 2016
We won't be taking this for M54 - it'll be fixed when M55 rolls out to stable early Dec. aelias@, should we mark this as fixed?
,
Oct 28 2016
Sounds good.
,
Nov 4 2016
[Automated comment] removing mislabelled merge-merged-2840
,
Nov 4 2016
[Automated comment] removing mislabelled merge-merged-2840
,
Jan 5 2017
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by aelias@chromium.org
, Oct 18 2016