New issue
Advanced search Search tips

Issue 699015 link

Starred by 1 user
Status: Fixed
Owner: ----
Closed: Apr 2017
Cc:
yosin@chromium.org
editing-dev@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Mouse click on <body> should cause one (not two) selectionchange

Reported by hu...@opera.com, Mar 7 2017 
This bug is not present in Firefox 52.0 (beta).

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

Steps:
1. out/Debug/content_shell -u http://logikbyran.se/editing/2.html
2. Click outside of the <input>-field.

Actual: Two selectionchange events.
Expected: One selectionchange event.

Bug discovered during https://codereview.chromium.org/2616623002 .
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 28 2017 

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

commit 31d55d991baf03aa9bc3ff5d5e67f709eee77959
Author: hugoh <hugoh@opera.com>
Date: Tue Mar 28 02:09:07 2017

Do not send redundant selectionchange-events (decouple focus)

Background:
Blink tells browser-side when a new <input>-element gets focus.
The information is passed in the
ViewHostMsg_TextInputStateChanged-message.

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:
When JavaScript moves focus to an element, element.focus(),
and when the user moves focus using tab-key navigation or
mouse, we don't clear the old selection (we hide it). This
means, we only send one selectionchange event, not two, for
each caret jump (as in Firefox).

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.

Follow-up will remove the remaining redundant clears:
 crbug.com/692898  (tab jumps to non-editable elements).

BUG= 678601 ,  679635 ,  699015 
TEST=In content_shell, select some text in an <input>-field,
     click another <input>-field (move focus).
     Notice: one selectionchange event is fired.

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

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

Comment 2 by bugdroid1@chromium.org, Mar 28 2017 

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

commit 914070a0e79c114aad81486aba7a3a15d94f590e
Author: imcheng <imcheng@chromium.org>
Date: Tue Mar 28 20:29:14 2017

Revert of Do not send redundant selectionchange-events (patchset #28 id:540001 of https://codereview.chromium.org/2616623002/ )

Reason for revert:
Broke webkit_tests: https://bugs.chromium.org/p/chromium/issues/detail?id=706119

Original issue's description:
> Do not send redundant selectionchange-events (decouple focus)
>
> Background:
> Blink tells browser-side when a new <input>-element gets focus.
> The information is passed in the
> ViewHostMsg_TextInputStateChanged-message.
>
> 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:
> When JavaScript moves focus to an element, element.focus(),
> and when the user moves focus using tab-key navigation or
> mouse, we don't clear the old selection (we hide it). This
> means, we only send one selectionchange event, not two, for
> each caret jump (as in Firefox).
>
> 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.
>
> Follow-up will remove the remaining redundant clears:
>  crbug.com/692898  (tab jumps to non-editable elements).
>
> BUG= 678601 ,  679635 ,  699015 
> TEST=In content_shell, select some text in an <input>-field,
>      click another <input>-field (move focus).
>      Notice: one selectionchange event is fired.
>
> Review-Url: https://codereview.chromium.org/2616623002
> Cr-Commit-Position: refs/heads/master@{#459984}
> Committed: https://chromium.googlesource.com/chromium/src/+/31d55d991baf03aa9bc3ff5d5e67f709eee77959

TBR=aelias@chromium.org,amaralp@chromium.org,bokan@chromium.org,kochi@chromium.org,yosin@chromium.org,hugoh@opera.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 678601 ,  679635 ,  699015 

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

[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/content/test/data/android/input/input_forms.html
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/LayoutTests/editing/input/keyboard_event_without_focus.html
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/LayoutTests/editing/selection/focus-and-display-none-and-redisplay-expected.txt
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/LayoutTests/editing/selection/focus-and-display-none-and-redisplay.html
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/LayoutTests/fast/events/touch/gesture/focus-selectionchange-on-tap-expected.txt
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/LayoutTests/fast/forms/focus-selection-input-expected.txt
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/LayoutTests/fast/forms/focus-selection-textarea-expected.txt
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/LayoutTests/platform/android/fast/forms/textarea/textarea-scrolled-mask-expected.txt
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/LayoutTests/platform/linux/fast/forms/focus-selection-textarea-expected.txt
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/LayoutTests/platform/linux/fast/forms/textarea/textarea-scrolled-mask-expected.txt
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/LayoutTests/platform/linux/plugins/mouse-click-plugin-clears-selection-expected.txt
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/LayoutTests/platform/mac-mac10.10/fast/forms/textarea/textarea-scrolled-mask-expected.txt
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/LayoutTests/platform/mac-mac10.10/plugins/mouse-click-plugin-clears-selection-expected.txt
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/LayoutTests/platform/mac-mac10.9/fast/forms/textarea/textarea-scrolled-mask-expected.txt
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/LayoutTests/platform/mac-mac10.9/plugins/mouse-click-plugin-clears-selection-expected.txt
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/LayoutTests/platform/mac/fast/forms/focus-selection-textarea-expected.txt
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/LayoutTests/platform/mac/fast/forms/textarea/textarea-scrolled-mask-expected.txt
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/LayoutTests/platform/mac/plugins/mouse-click-plugin-clears-selection-expected.txt
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/LayoutTests/platform/win/fast/forms/textarea/textarea-scrolled-mask-expected.txt
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/LayoutTests/platform/win/plugins/mouse-click-plugin-clears-selection-expected.txt
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/Source/core/editing/EditingUtilities.cpp
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/Source/core/editing/EditingUtilities.h
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/Source/core/editing/PendingSelection.cpp
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/Source/core/page/FocusController.cpp
[modify] https://crrev.com/914070a0e79c114aad81486aba7a3a15d94f590e/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
[delete] https://crrev.com/d406a54b2a677132a79f9da2c9624bb8c3ab07c8/third_party/WebKit/Source/web/tests/data/editable_elements.html
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 29 2017 

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

commit 92bf3c29fccac551acceff254e53d7ea73367158
Author: hugoh <hugoh@opera.com>
Date: Wed Mar 29 07:18:54 2017

Reland: Do not send redundant selectionchange-events (decouple focus)

Reason for reland:
Update Win7/10 LayoutTests correctly:  crbug.com/706119 

Background:
Blink tells browser-side when a new <input>-element gets focus.
The information is passed in the
ViewHostMsg_TextInputStateChanged-message.

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:
When JavaScript moves focus to an element, element.focus(),
and when the user moves focus using tab-key navigation or
mouse, we don't clear the old selection (we hide it). This
means, we only send one selectionchange event, not two, for
each caret jump (as in Firefox).

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.

Follow-up will remove the remaining redundant clears:
 crbug.com/692898  (tab jumps to non-editable elements).

BUG= 678601 ,  679635 ,  699015 
TEST=In content_shell, select some text in an <input>-field,
     click another <input>-field (move focus).
     Notice: one selectionchange event is fired.

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

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

Comment 4 by bugdroid1@chromium.org, Apr 1 2017 

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

commit 1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065
Author: aelias <aelias@chromium.org>
Date: Sat Apr 01 00:00:23 2017

Revert of Reland: Do not send redundant selectionchange-events (patchset #29 id:560001 of https://codereview.chromium.org/2616623002/ )

Reason for revert:
Caused  http://crbug.com/707156 , please reland after fixing it.

BUG= 707156 

Original issue's description:
> Reland: Do not send redundant selectionchange-events (decouple focus)
>
> Reason for reland:
> Update Win7/10 LayoutTests correctly:  crbug.com/706119 
>
> Background:
> Blink tells browser-side when a new <input>-element gets focus.
> The information is passed in the
> ViewHostMsg_TextInputStateChanged-message.
>
> 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:
> When JavaScript moves focus to an element, element.focus(),
> and when the user moves focus using tab-key navigation or
> mouse, we don't clear the old selection (we hide it). This
> means, we only send one selectionchange event, not two, for
> each caret jump (as in Firefox).
>
> 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.
>
> Follow-up will remove the remaining redundant clears:
>  crbug.com/692898  (tab jumps to non-editable elements).
>
> BUG= 678601 ,  679635 ,  699015 
> TEST=In content_shell, select some text in an <input>-field,
>      click another <input>-field (move focus).
>      Notice: one selectionchange event is fired.
>
> Review-Url: https://codereview.chromium.org/2616623002
> Cr-Commit-Position: refs/heads/master@{#460314}
> Committed: https://chromium.googlesource.com/chromium/src/+/92bf3c29fccac551acceff254e53d7ea73367158

TBR=amaralp@chromium.org,bokan@chromium.org,kochi@chromium.org,yosin@chromium.org,hugoh@opera.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 678601 ,  679635 ,  699015 

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

[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/content/test/data/android/input/input_forms.html
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/third_party/WebKit/LayoutTests/editing/input/keyboard_event_without_focus.html
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/third_party/WebKit/LayoutTests/editing/selection/focus-and-display-none-and-redisplay-expected.txt
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/third_party/WebKit/LayoutTests/editing/selection/focus-and-display-none-and-redisplay.html
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/third_party/WebKit/LayoutTests/fast/events/touch/gesture/focus-selectionchange-on-tap-expected.txt
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/third_party/WebKit/LayoutTests/fast/forms/focus-selection-input-expected.txt
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/third_party/WebKit/LayoutTests/platform/android/fast/forms/textarea/textarea-scrolled-mask-expected.txt
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/third_party/WebKit/LayoutTests/platform/linux/fast/forms/textarea/textarea-scrolled-mask-expected.txt
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/third_party/WebKit/LayoutTests/platform/linux/plugins/mouse-click-plugin-clears-selection-expected.txt
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/third_party/WebKit/LayoutTests/platform/mac-mac10.10/fast/forms/textarea/textarea-scrolled-mask-expected.txt
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/third_party/WebKit/LayoutTests/platform/mac-mac10.10/plugins/mouse-click-plugin-clears-selection-expected.txt
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/third_party/WebKit/LayoutTests/platform/mac-mac10.9/fast/forms/textarea/textarea-scrolled-mask-expected.txt
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/third_party/WebKit/LayoutTests/platform/mac-mac10.9/plugins/mouse-click-plugin-clears-selection-expected.txt
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/third_party/WebKit/LayoutTests/platform/mac/fast/forms/textarea/textarea-scrolled-mask-expected.txt
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/third_party/WebKit/LayoutTests/platform/mac/plugins/mouse-click-plugin-clears-selection-expected.txt
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/third_party/WebKit/LayoutTests/platform/win/fast/forms/textarea/textarea-scrolled-mask-expected.txt
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/third_party/WebKit/LayoutTests/platform/win/plugins/mouse-click-plugin-clears-selection-expected.txt
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/third_party/WebKit/Source/core/editing/EditingUtilities.cpp
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/third_party/WebKit/Source/core/editing/EditingUtilities.h
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/third_party/WebKit/Source/core/editing/PendingSelection.cpp
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/third_party/WebKit/Source/core/page/FocusController.cpp
[modify] https://crrev.com/1fe7a105adbf7d7e58b3fcb38aefe2fd735c6065/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
[delete] https://crrev.com/b4c713070f5634b5ea05a677bd0160370c776d7a/third_party/WebKit/Source/web/tests/data/editable_elements.html
Project Member

Comment 5 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 6 by hu...@opera.com, Apr 25 2017

Status: Fixed (was: Available)
►

Sign in to add a comment