Correct logic "Should ContextMenu target the selection?"

third_party/WebKit/Source/core/input/EventHandler.cpp

Index: third_party/WebKit/Source/core/input/EventHandler.cpp
diff --git a/third_party/WebKit/Source/core/input/EventHandler.cpp b/third_party/WebKit/Source/core/input/EventHandler.cpp
index dc72afdad9c6949bd47ce1ce91fbc5316c683ffc..5626715588ade3e38a15c1a2a6b4691824dab9a8 100644
--- a/third_party/WebKit/Source/core/input/EventHandler.cpp
+++ b/third_party/WebKit/Source/core/input/EventHandler.cpp
@@ -1824,14 +1824,13 @@ WebInputEventResult EventHandler::SendContextMenuEventForKey(
   Element* focused_element =
       override_target_element ? override_target_element : doc->FocusedElement();
   FrameSelection& selection = frame_->Selection();
-  Position start =
-      selection.ComputeVisibleSelectionInDOMTreeDeprecated().Start();
   VisualViewport& visual_viewport = frame_->GetPage()->GetVisualViewport();
 
-  if (!override_target_element && start.AnchorNode() && !selection.IsHidden() &&

[Xiaocheng, 2017/05/17 02:06:27]

The current impl is incorrect. When we have an unfocused range selection in
contenteditable, the context menu is still targeted at the selection instead of
the focused element.

We should check FS::SelectionHasFocus() here instead of FS::IsHidden().

FS::IsHidden() should only be used by paint code. That's why I insisted having
two separate functions HasFocus() and IsHidden() to serve different purposes at
the beginning, instead of one IsActive().

[hugoh_UTC2, 2017/05/17 09:07:47]

Thanks. You're right. I fixed this in PS3.

-      (selection.ComputeVisibleSelectionInDOMTreeDeprecated()
-           .RootEditableElement() ||
-       selection.ComputeVisibleSelectionInDOMTreeDeprecated().IsRange())) {
+  if (!override_target_element &&
+      (selection.ComputeVisibleSelectionInDOMTreeDeprecated().IsRange() ||

[yosin_UTC9, 2017/05/17 01:54:59]

I would like to move this condition expression, except for
|!override_target_element|,
into named function, e.g. ShouldShowContextMenuAtSelection(const LocalFrame&) ->
bool.
To help to avoid to use ComputeVisibleSelectionInDOMTreeDeprecated() easier.

[hugoh_UTC2, 2017/05/17 09:07:47]

Done, see PS3.

+       selection.ComputeVisibleSelectionInDOMTreeDeprecated()
+           .RootEditableElement()) &&
+      !selection.IsHidden()) {
     // TODO(editing-dev): Use of updateStyleAndLayoutIgnorePendingStylesheets
     // needs to be audited.  See http://crbug.com/590369 for more details.
     doc->UpdateStyleAndLayoutIgnorePendingStylesheets();