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 1804 matching lines @@
 #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 && 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();
 
     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