New issue
Advanced search Search tips

Issue 692898 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Focus jump from <input> to non cursor-element should not clear selection

Reported by hu...@opera.com, Feb 16 2017

Issue description

<script>
document.addEventListener("selectionchange", function() {
  const selection = getSelection();
  console.log("Selection changed!");
});
</script>
<input autofocus>
<button>

Steps:
1. out/Debug/content_shell -u http://logikbyran.se/editing/1.html
2. Hit Tab-key on your keyboard (focus moves to <button>).

Actual: 1 selectionchange event (selection is cleared).
Expected: 0 selectionchange events and selection looks cleared (not painted). 

Note on web compatibility:
A conflicting behavior is found in Firefox 52.0 (beta). Difference is, Firefox sends 2 (identical?) selectionchange events that say the selection (here the invisible cursor) moved to the button. So to align with Firefox, we could also image:
 Expected*: 1 selectionchange event (selection follows Tab-key's focus move to the <button>).
 

Comment 1 by hu...@opera.com, Mar 7 2017

Description: Show this description
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0f65d25a4097959d977dbb1077717323438240a6

commit 0f65d25a4097959d977dbb1077717323438240a6
Author: hugoh <hugoh@opera.com>
Date: Fri Apr 14 07:10:29 2017

Do not send redundant selectionchange-events (decouple focus)

This CL aims to remove redundant selectionchange-events
that were sent upon change of focus caused by element.focus(),
tab-navigation, spatnav and mouse-clicks.

Goals:
1. Send == one selectionchange-event, not two, for each caret jump.
2. Send <= one selectionchange-event, not two, for each focus jump.

Background:
When you click/tab to an <input> text-field, a
ViewHostMsg_TextInputStateChanged-message is sent to browser-side.

Problem:
With current logic, RenderFrameImpl::didChangeSelection is
called twice so two ViewHostMsg_TextInputStateChanged-messages
are sent to browser-side:
 (1) when focus leaves an <input>-field (unnecessary!).
 (2) when focus enters another <input>-field.

Worse, also the web page gets two selectionchange events.
The first one is immediately invalid so the webpage should
not react to it.

(1) happens because FocusController::setFocusedElement()
always clears the selection when a new element gets focus.

Solution:
Do not clear selection when focus moves. To keep current visual
behavior when focus moves away from a text-field we need to hide
that field's selection (clicking outside a text-field hides its
selection).

Test updates:
1. Check for one selectionchange event, not two.
2. LayoutTests' trees now expect the "hidden" selection.
3. A new test in WebFrameTest.cpp tests tab-key navigation.

BUG= 678601 ,  679635 ,  699015 ,  692898 
TEST=In content_shell, select some text in an <input>-field,
     click another <input>-field (move focus).
     Notice: one selectionchange event is fired (as in Firefox).
TEST=In content_shell, select some text in an <input>-field,
     click on an <img>. Notice: selection gets hid and
     zero selectionchange events are fired (as in Firefox).

Review-Url: https://codereview.chromium.org/2616623002
Cr-Commit-Position: refs/heads/master@{#464698}

[modify] https://crrev.com/0f65d25a4097959d977dbb1077717323438240a6/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
[modify] https://crrev.com/0f65d25a4097959d977dbb1077717323438240a6/content/test/data/android/input/input_forms.html
[modify] https://crrev.com/0f65d25a4097959d977dbb1077717323438240a6/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/0f65d25a4097959d977dbb1077717323438240a6/third_party/WebKit/LayoutTests/editing/input/keyboard_event_without_focus.html
[modify] https://crrev.com/0f65d25a4097959d977dbb1077717323438240a6/third_party/WebKit/LayoutTests/editing/selection/focus-and-display-none-and-redisplay-expected.txt
[modify] https://crrev.com/0f65d25a4097959d977dbb1077717323438240a6/third_party/WebKit/LayoutTests/editing/selection/focus-and-display-none-and-redisplay.html
[modify] https://crrev.com/0f65d25a4097959d977dbb1077717323438240a6/third_party/WebKit/LayoutTests/fast/events/touch/gesture/focus-selectionchange-on-tap-expected.txt
[modify] https://crrev.com/0f65d25a4097959d977dbb1077717323438240a6/third_party/WebKit/LayoutTests/fast/forms/focus-selection-input-expected.txt
[modify] https://crrev.com/0f65d25a4097959d977dbb1077717323438240a6/third_party/WebKit/LayoutTests/fast/forms/focus-selection-textarea-expected.txt
[modify] https://crrev.com/0f65d25a4097959d977dbb1077717323438240a6/third_party/WebKit/LayoutTests/platform/android/fast/forms/textarea/textarea-scrolled-mask-expected.txt
[modify] https://crrev.com/0f65d25a4097959d977dbb1077717323438240a6/third_party/WebKit/LayoutTests/platform/linux/fast/forms/textarea/textarea-scrolled-mask-expected.txt
[modify] https://crrev.com/0f65d25a4097959d977dbb1077717323438240a6/third_party/WebKit/LayoutTests/platform/mac-mac10.10/fast/forms/textarea/textarea-scrolled-mask-expected.txt
[modify] https://crrev.com/0f65d25a4097959d977dbb1077717323438240a6/third_party/WebKit/LayoutTests/platform/mac-mac10.9/fast/forms/textarea/textarea-scrolled-mask-expected.txt
[modify] https://crrev.com/0f65d25a4097959d977dbb1077717323438240a6/third_party/WebKit/LayoutTests/platform/mac/fast/forms/textarea/textarea-scrolled-mask-expected.txt
[modify] https://crrev.com/0f65d25a4097959d977dbb1077717323438240a6/third_party/WebKit/LayoutTests/platform/win/fast/forms/textarea/textarea-scrolled-mask-expected.txt
[modify] https://crrev.com/0f65d25a4097959d977dbb1077717323438240a6/third_party/WebKit/Source/core/editing/FrameSelection.cpp
[modify] https://crrev.com/0f65d25a4097959d977dbb1077717323438240a6/third_party/WebKit/Source/core/editing/FrameSelection.h
[modify] https://crrev.com/0f65d25a4097959d977dbb1077717323438240a6/third_party/WebKit/Source/core/editing/LayoutSelection.cpp
[modify] https://crrev.com/0f65d25a4097959d977dbb1077717323438240a6/third_party/WebKit/Source/core/editing/LayoutSelection.h
[modify] https://crrev.com/0f65d25a4097959d977dbb1077717323438240a6/third_party/WebKit/Source/core/page/FocusController.cpp
[modify] https://crrev.com/0f65d25a4097959d977dbb1077717323438240a6/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
[add] https://crrev.com/0f65d25a4097959d977dbb1077717323438240a6/third_party/WebKit/Source/web/tests/data/editable_elements.html

Comment 3 by hu...@opera.com, Apr 25 2017

Status: Fixed (was: Available)

Sign in to add a comment