Skip to content

GitLab

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

Color control popups should be dismissed if the input type changes 

Currently, if the color picker or color suggestion picker is open and
the type of the owning input element changes from color to something
else, the popup to choose a new color remains open even though the type
of the input has changed. This can be seen in the
color-picker-appearance-set-type test, when run with the form controls
refresh flag enabled. This change is fixing this issue, which results
in the popup being dismissed when the input's type is changed.

This behavior is not spec'ed and neither is there consistency between
browsers on how this scenario should behave - Firefox does not dismiss
popups on type change, while Edgehtml-based Edge does. However, from a
user's perspective, it is strange to see the old popup once the type of
the input has changed. Additionally, in the case of the date control,
the picker does get dismissed when a type change occurs. Hence, it
makes sense to do the same for the color control, so that there is
consistent behavior across controls in Chromium.

Note that this change is not limited to the form controls refresh flag
as the color suggestion picker, which pre-dates the form controls
refresh project, is also being fixed. A test is being added for this
case to validate the new behavior.

Bug: 985889
Change-Id: Idcf7c5ad056b21bf59870496a8fe618ee2309d05
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1759226Reviewed-by: default avatarKent Tamura <tkent@chromium.org>
Reviewed-by: default avatarMason Freed <masonfreed@chromium.org>
Commit-Queue: Sanket Joshi <sajos@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#688558}
parent 2244f088
Hide whitespace changes
Inline Side-by-side
Showing with 61 additions and 3 deletions
third_party/blink/renderer/core/html/forms/html_input_element.cc
View file @ d755d033
......@@ -409,6 +409,7 @@ void HTMLInputElement::UpdateType() {
input_type_->ShouldRespectHeightAndWidthAttributes();
bool could_be_successful_submit_button = CanBeSuccessfulSubmitButton();
input_type_view_->ClosePopupView();
input_type_view_->DestroyShadowSubtree();
DropInnerEditorElement();
SetForceReattachLayoutTree();
......
third_party/blink/web_tests/fast/forms/color/color-suggestion-picker-appearance-set-type.html 0 → 100644
View file @ d755d033
<!DOCTYPE html>
<html>
<head>
<head>
<script>
testRunner.dumpAsText();
testRunner.waitUntilDone();
</script>
<script src='../resources/picker-common.js'></script>
</head>
<body>
<input type='color' id='color' value='#ff0000' list>
<p id='description' style='opacity: 0'></p>
<div id='console' style='opacity: 0'></div>
<script>
let descriptionContainer = document.getElementById('description');
var colorControl = document.getElementById('color');
openPicker(colorControl, openPickerCallback);
function openPickerCallback() {
// The color suggestion picker popup should close when the input type changes.
colorControl.type = 'text';
if (internals.pagePopupWindow) {
descriptionContainer.append('Popup did not close when the input type changed.');
} else {
descriptionContainer.append('Popup closed when the input type changed.');
}
descriptionContainer.append(document.createElement('br'), 'TEST COMPLETE');
testRunner.notifyDone();
}
</script>
</body>
</html>
\ No newline at end of file
third_party/blink/web_tests/fast/forms/controls-new-ui/color/color-picker-appearance-set-type.html
View file @ d755d033
......@@ -3,10 +3,10 @@
<head>
<head>
<script>
testRunner.setUseMockTheme(false);
testRunner.dumpAsText();
testRunner.waitUntilDone();
</script>
<script src='../../../forms/resources/picker-common.js'></script>
<script src='../../resources/picker-common.js'></script>
</head>
<body>
<input type='color' id='color'>
......@@ -15,11 +15,26 @@ testRunner.waitUntilDone();
<div id='console' style='opacity: 0'></div>
<script>
let descriptionContainer = document.getElementById('description');
var colorControl = document.getElementById('color');
openPicker(colorControl, openPickerCallback, openPickerCallback);
function openPickerCallback() {
colorControl.type = 'text';
if (internals.pagePopupWindow) {
descriptionContainer.append('Popup opened.', document.createElement('br'));
// The color picker popup should close when the input type changes.
colorControl.type = 'text';
if (internals.pagePopupWindow) {
descriptionContainer.append('Popup did not close when the input type changed.');
} else {
descriptionContainer.append('Popup closed when the input type changed.');
}
} else {
descriptionContainer.append('Popup did not open.');
}
descriptionContainer.append(document.createElement('br'), 'TEST COMPLETE');
testRunner.notifyDone();
}
</script>
......
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