[Smart Selection] reset is not properly logged sometimes. |
|||
Issue descriptionWhile debugging with issue 787370, I found that sometimes reset is not get logged. By taking a closer look, this is because in SelectionController, we are calling ShowNonLocatedContextMenu() to trigger selection menu, which will generate a pair of mock coordinates according to the selection at that point. However, in ContextMenuClient::ShowContextMenu(), we are using this coordinates to check if it's inside current selection. Unfortunately, that doesn't work correctly sometimes. It affects reset logging only.
,
Nov 22 2017
BTW, I tried more address, only one of them has this problem in Chrome, same webpage in WebView Shell doesn't repro. Copy paste this to gmail, doesn't repro either.
,
Nov 22 2017
,
Nov 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c19bd1dbd9bf350b63f7835f4a80ebdba3b44684 commit c19bd1dbd9bf350b63f7835f4a80ebdba3b44684 Author: Shimi Zhang <ctzsm@chromium.org> Date: Thu Nov 23 20:55:23 2017 [Smart Selection] Fix Reset logging issue Sometimes Smart Selection logging is not triggered because we false detected that there is no selection. ShowNonLocatedContextMenu() is making coordinates according to current selection rect. However, sometimes that doesn't satisfy a check later on in ContextMenuClient::ShowContextMenu. In that check, We converted coordinates to character position in current element then compare with selection range. We need to make sure the coordinates are in the current selection range, this CL checks if the current ShowContextMenu call is from Smart Selection reset, means it passed the previous inside check already. Bug: 787665 Change-Id: I6c6792d54f6f205644719b68f7602e6b4332b056 Reviewed-on: https://chromium-review.googlesource.com/784129 Reviewed-by: Dave Tapuska <dtapuska@chromium.org> Reviewed-by: Changwan Ryu <changwan@chromium.org> Commit-Queue: Changwan Ryu <changwan@chromium.org> Cr-Commit-Position: refs/heads/master@{#519020} [modify] https://crrev.com/c19bd1dbd9bf350b63f7835f4a80ebdba3b44684/third_party/WebKit/Source/core/page/ContextMenuClient.cpp
,
Dec 4 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by ctzsm@chromium.org
, Nov 22 2017