Skip to content

GitLab

T
tangled
Commit ca933728 authored by Sanket Joshi's avatar Sanket Joshi Committed by Commit Bot
Browse files
Options

Color picker: Tabbing into inputs should not insert tab characters 

When the tab keydown event occurs while a keyboard focusable element
has focus, the tab keypress event should be suppressed. Inside a popup
window though, keypress does still get fired today. This CL fixes that.

Bug: 1011168
Change-Id: I45f0853a00fc9ea45777fc0d7cdafbdb6e55eeda
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1842951
Commit-Queue: Sanket Joshi <sajos@microsoft.com>
Reviewed-by: default avatarKent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703403}
parent 6806b2f9
Hide whitespace changes
Inline Side-by-side
Showing with 79 additions and 0 deletions
third_party/blink/renderer/core/exported/web_page_popup_impl.cc
View file @ ca933728
......@@ -38,6 +38,7 @@
#include "third_party/blink/public/web/web_view_client.h"
#include "third_party/blink/renderer/core/accessibility/ax_object_cache_base.h"
#include "third_party/blink/renderer/core/dom/context_features.h"
#include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/dom/events/event_dispatch_forbidden_scope.h"
#include "third_party/blink/renderer/core/events/message_event.h"
#include "third_party/blink/renderer/core/events/web_input_event_conversion.h"
......@@ -66,6 +67,7 @@
#include "third_party/blink/renderer/platform/heap/handle.h"
#include "third_party/blink/renderer/platform/heap/heap.h"
#include "third_party/blink/renderer/platform/instrumentation/tracing/trace_event.h"
#include "third_party/blink/renderer/platform/keyboard_codes.h"
#include "third_party/blink/renderer/platform/web_test_support.h"
namespace blink {
......@@ -406,11 +408,25 @@ WebInputEventResult WebPagePopupImpl::HandleKeyEvent(
const WebKeyboardEvent& event) {
if (closing_)
return WebInputEventResult::kNotHandled;
if (WebInputEvent::kRawKeyDown == event.GetType()) {
Element* focused_element = FocusedElement();
if (focused_element && focused_element->IsKeyboardFocusable() &&
event.windows_key_code == VKEY_TAB) {
// If the tab key is pressed while a keyboard focusable element is
// focused, we should not send a corresponding keypress event.
suppress_next_keypress_event_ = true;
}
}
return MainFrame().GetEventHandler().KeyEvent(event);
}
WebInputEventResult WebPagePopupImpl::HandleCharEvent(
const WebKeyboardEvent& event) {
if (suppress_next_keypress_event_) {
suppress_next_keypress_event_ = false;
return WebInputEventResult::kHandledSuppressed;
}
return HandleKeyEvent(event);
}
......@@ -455,6 +471,21 @@ LocalFrame& WebPagePopupImpl::MainFrame() const {
return *To<LocalFrame>(page_->MainFrame());
}
Element* WebPagePopupImpl::FocusedElement() const {
if (!page_)
return nullptr;
LocalFrame* frame = page_->GetFocusController().FocusedFrame();
if (!frame)
return nullptr;
Document* document = frame->GetDocument();
if (!document)
return nullptr;
return document->FocusedElement();
}
bool WebPagePopupImpl::IsViewportPointInWindow(int x, int y) {
WebRect point_in_window(x, y, 0, 0);
WidgetClient()->ConvertViewportToWindow(&point_in_window);
......
third_party/blink/renderer/core/exported/web_page_popup_impl.h
View file @ ca933728
......@@ -44,6 +44,7 @@ class Layer;
}
namespace blink {
class Element;
class Page;
class PagePopupChromeClient;
class PagePopupClient;
......@@ -138,6 +139,8 @@ class CORE_EXPORT WebPagePopupImpl final : public WebPagePopup,
// This may only be called if page_ is non-null.
LocalFrame& MainFrame() const;
Element* FocusedElement() const;
bool IsViewportPointInWindow(int x, int y);
// PagePopup function
......@@ -167,6 +170,8 @@ class CORE_EXPORT WebPagePopupImpl final : public WebPagePopup,
base::TimeTicks raf_aligned_input_start_time_;
bool is_accelerated_compositing_active_ = false;
bool suppress_next_keypress_event_ = false;
friend class WebPagePopup;
friend class PagePopupChromeClient;
......
third_party/blink/web_tests/fast/forms/controls-new-ui/color/color-picker-no-tab-character.html 0 → 100644
View file @ ca933728
<!DOCTYPE html>
<html>
<head>
<script src='../../../../resources/testharness.js'></script>
<script src='../../../../resources/testharnessreport.js'></script>
<script src='../../../forms/resources/picker-common.js'></script>
</head>
<body>
<input type='color' id='color' value='#80d9ff'>
<script>
'use strict';
let t = async_test('Color picker: Tabbing into a color value container should not insert a tab character.');
t.step(() => {
let colorControl = document.getElementById('color');
openPicker(colorControl, t.step_func_done(() => {
popupWindow.focus();
const popupDocument = popupWindow.document;
const rValueContainer = popupDocument.getElementById('rValueContainer');
const gValueContainer = popupDocument.getElementById('gValueContainer');
const bValueContainer = popupDocument.getElementById('bValueContainer');
assert_equals(rValueContainer.value, '128');
assert_equals(gValueContainer.value, '217');
assert_equals(bValueContainer.value, '255');
const hueSliderSelectionRing = popupDocument.querySelector('hue-slider > color-selection-ring');
hueSliderSelectionRing.focus();
assert_equals(popupDocument.activeElement, hueSliderSelectionRing);
eventSender.keyDown('Tab');
assert_equals(popupDocument.activeElement, rValueContainer, 'rValueContainer should be the active element');
assert_equals(rValueContainer.value, '128', 'rValueContainer\'s value should not have changed');
eventSender.keyDown('Tab');
assert_equals(popupDocument.activeElement, gValueContainer, 'gValueContainer should be the active element');
assert_equals(gValueContainer.value, '217', 'gValueContainer\'s value should not have changed');
eventSender.keyDown('Tab');
assert_equals(popupDocument.activeElement, bValueContainer, 'bValueContainer should be the active element');
assert_equals(bValueContainer.value, '255', 'bValueContainer\'s value should not have changed');
}), t.step_func_done(() => {
assert_false(internals.runtimeFlags.formControlsRefreshEnabled, "Popup should only not open when the formControlsRefresh flag is disabled.");
}));
});
</script>
</body>
</html>
\ No newline at end of file
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment