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 b370359ab3bf0239068c6a8ebe2af40b81af12f5..578ec4045547af2373c21a96415ddf682dbafe0b 100644
--- a/third_party/WebKit/Source/core/input/EventHandler.cpp
+++ b/third_party/WebKit/Source/core/input/EventHandler.cpp
@@ -1822,14 +1822,9 @@ 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() &&
-      (selection.ComputeVisibleSelectionInDOMTreeDeprecated()
-           .RootEditableElement() ||
-       selection.ComputeVisibleSelectionInDOMTreeDeprecated().IsRange())) {
+  if (!override_target_element && selection.SelectionHasFocus()) {

[yosin_UTC9, 2017/05/16 09:45:22]

It seems |start.AnchorNode() && !selection.IsHidden()| means
selection.IsRange().
But, SelectionHasFocus() return true if start position is in focused element.

Hence, in case of caret selection in focused non-editable, this patch executes
then-clause,
where original code doesn't.

Note: this new behavior is as same as before [1] unless considering focus.

WDYT?


[1] http://crrev.com/2869713003 Make context menu aware of hidden selection

[hugoh_UTC2, 2017/05/16 12:48:14]

Right! Context menu should not use a hidden caret selection as context... PS1
was too optimistic.

In PS2 I dropped the redundant start.AnchorNode() and moved the slower
IsHidden() to the end. I also moved the quick IsRange() before the slower
RootEditableElement().

     // TODO(editing-dev): Use of updateStyleAndLayoutIgnorePendingStylesheets
     // needs to be audited.  See http://crbug.com/590369 for more details.
     doc->UpdateStyleAndLayoutIgnorePendingStylesheets();