Correct logic "Should ContextMenu target the selection?"

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

 /*
  * Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights
  * reserved.
  * Copyright (C) 2006 Alexey Proskuryakov (ap@webkit.org)
  * Copyright (C) 2012 Digia Plc. and/or its subsidiary(-ies)
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
@@ skipping 1784 matching lines @@
 
   GetSelectionController().SendContextMenuEvent(mev, position_in_contents);
 
   Node* target_node =
       override_target_node ? override_target_node : mev.InnerNode();
   return mouse_event_manager_->DispatchMouseEvent(
       UpdateMouseEventTargetNode(target_node), EventTypeNames::contextmenu,
       event, mev.GetHitTestResult().CanvasRegionId(), 0);
 }
 
+static bool ShouldShowContextMenuAtSelection(const LocalFrame& frame) {

[Xiaocheng, 2017/05/17 19:20:26]

nit: I prefer passing FrameSelection to reduce a call of LocalFrame::Selection()

[yosin_UTC9, 2017/05/18 01:17:15]

me too.

[hugoh_UTC2, 2017/05/22 12:26:12]

Done.

+  FrameSelection& selection = frame.Selection();
+  VisibleSelection visible_selection =
+      selection.ComputeVisibleSelectionInDOMTreeDeprecated();
+  if (!visible_selection.IsRange() && !visible_selection.RootEditableElement())
+    return false;
+

[yosin_UTC9, 2017/05/18 01:17:15]

nit: Please remove an extra blank line.

[hugoh_UTC2, 2017/05/22 12:26:12]

Done.

+  return selection.SelectionHasFocus();
+}
+
 WebInputEventResult EventHandler::SendContextMenuEventForKey(
     Element* override_target_element) {
   FrameView* view = frame_->View();
   if (!view)
     return WebInputEventResult::kNotHandled;
 
   Document* doc = frame_->GetDocument();
   if (!doc)
     return WebInputEventResult::kNotHandled;
 
   static const int kContextMenuMargin = 1;
 
 #if OS(WIN)
   int right_aligned = ::GetSystemMetrics(SM_MENUDROPALIGNMENT);
 #else
   int right_aligned = 0;
 #endif
   IntPoint location_in_root_frame;
 
   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 && ShouldShowContextMenuAtSelection(*frame_)) {
     // TODO(editing-dev): Use of updateStyleAndLayoutIgnorePendingStylesheets
     // needs to be audited.  See http://crbug.com/590369 for more details.
     doc->UpdateStyleAndLayoutIgnorePendingStylesheets();
 
     IntRect first_rect = frame_->GetEditor().FirstRectForRange(
         selection.ComputeVisibleSelectionInDOMTree()
             .ToNormalizedEphemeralRange());
 
     int x = right_aligned ? first_rect.MaxX() : first_rect.X();
     // In a multiline edit, firstRect.maxY() would end up on the next line, so
@@ skipping 274 matching lines @@
     MouseEventWithHitTestResults& mev,
     LocalFrame* subframe) {
   WebInputEventResult result =
       subframe->GetEventHandler().HandleMouseReleaseEvent(mev.Event());
   if (result != WebInputEventResult::kNotHandled)
     return result;
   return WebInputEventResult::kHandledSystem;
 }
 
 }  // namespace blink