Skip to content

Commit

Permalink
[inert] Remove hit test retargeting for inert
Browse files Browse the repository at this point in the history
r942180 made inert nodes behave as 'pointer-events: none', as resolved
by the CSSWG in w3c/csswg-drafts#6685

Then, the magic hit test retargeting for inert is no longer needed.

Bug: 692360
Change-Id: I950f0967f093d6acb20877e049fb1bcf75a88de1
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3257651
Commit-Queue: Oriol Brufau <obrufau@igalia.com>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#942823}
NOKEYCHECK=True
GitOrigin-RevId: e2b9c07caf767a295026023ba5813ca339024a76
  • Loading branch information
Loirooriol authored and copybara-github committed Nov 18, 2021
1 parent da73063 commit 440e5c9
Show file tree
Hide file tree
Showing 18 changed files with 35 additions and 114 deletions.
6 changes: 2 additions & 4 deletions blink/renderer/core/editing/selection_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1031,8 +1031,7 @@ void SelectionController::HandleMouseDraggedEvent(
if (!Selection().IsAvailable())
return;
if (selection_state_ != SelectionState::kExtendedSelection) {
HitTestRequest request(HitTestRequest::kReadOnly | HitTestRequest::kActive |
HitTestRequest::kRetargetForInert);
HitTestRequest request(HitTestRequest::kReadOnly | HitTestRequest::kActive);
HitTestLocation location(mouse_down_pos);
HitTestResult result(request, location);
frame_->GetDocument()->GetLayoutView()->HitTest(location, result);
Expand All @@ -1055,8 +1054,7 @@ void SelectionController::UpdateSelectionForMouseDrag(
return;

HitTestRequest request(HitTestRequest::kReadOnly | HitTestRequest::kActive |
HitTestRequest::kMove |
HitTestRequest::kRetargetForInert);
HitTestRequest::kMove);
HitTestLocation location(last_known_mouse_position_in_root_frame);
HitTestResult result(request, location);
layout_view->HitTest(location, result);
Expand Down
3 changes: 1 addition & 2 deletions blink/renderer/core/editing/visible_units.cc
Original file line number Diff line number Diff line change
Expand Up @@ -441,8 +441,7 @@ PositionWithAffinity PositionForContentsPointRespectingEditingBoundary(
LocalFrame* frame) {
HitTestRequest request = HitTestRequest::kMove | HitTestRequest::kReadOnly |
HitTestRequest::kActive |
HitTestRequest::kIgnoreClipping |
HitTestRequest::kRetargetForInert;
HitTestRequest::kIgnoreClipping;
HitTestLocation location(contents_point);
HitTestResult result(request, location);
frame->GetDocument()->GetLayoutView()->HitTest(location, result);
Expand Down
34 changes: 11 additions & 23 deletions blink/renderer/core/input/event_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -785,8 +785,7 @@ WebInputEventResult EventHandler::HandleMousePressEvent(
if (!frame_->View())
return WebInputEventResult::kNotHandled;

HitTestRequest request(HitTestRequest::kActive |
HitTestRequest::kRetargetForInert);
HitTestRequest request(HitTestRequest::kActive);
// Save the document point we generate in case the window coordinate is
// invalidated by what happens when we dispatch the event.
PhysicalOffset document_point = frame_->View()->ConvertFromRootFrame(
Expand Down Expand Up @@ -887,8 +886,7 @@ WebInputEventResult EventHandler::HandleMousePressEvent(
GetSelectionController().InitializeSelectionState();

HitTestResult hit_test_result = event_handling_util::HitTestResultInFrame(
frame_, HitTestLocation(document_point),
HitTestRequest::kReadOnly | HitTestRequest::kRetargetForInert);
frame_, HitTestLocation(document_point), HitTestRequest::kReadOnly);
InputDeviceCapabilities* source_capabilities =
frame_->DomWindow()->GetInputDeviceCapabilities()->FiresTouchEvents(
mouse_event.FromTouch());
Expand Down Expand Up @@ -916,8 +914,7 @@ WebInputEventResult EventHandler::HandleMousePressEvent(
if (event_result == WebInputEventResult::kNotHandled) {
if (ShouldRefetchEventTarget(mev)) {
HitTestRequest read_only_request(HitTestRequest::kReadOnly |
HitTestRequest::kActive |
HitTestRequest::kRetargetForInert);
HitTestRequest::kActive);
mev = frame_->GetDocument()->PerformMouseEventHitTest(
read_only_request, document_point, mouse_event);
}
Expand Down Expand Up @@ -1032,8 +1029,7 @@ WebInputEventResult EventHandler::HandleMouseMoveOrLeaveEvent(
return WebInputEventResult::kHandledSystem;
}

HitTestRequest::HitTestRequestType hit_type =
HitTestRequest::kMove | HitTestRequest::kRetargetForInert;
HitTestRequest::HitTestRequestType hit_type = HitTestRequest::kMove;
if (mouse_event_manager_->MousePressed()) {
hit_type |= HitTestRequest::kActive;
}
Expand Down Expand Up @@ -1203,8 +1199,7 @@ WebInputEventResult EventHandler::HandleMouseReleaseEvent(

// Mouse events simulated from touch should not hit-test again.
DCHECK(!mouse_event.FromTouch());
HitTestRequest::HitTestRequestType hit_type =
HitTestRequest::kRelease | HitTestRequest::kRetargetForInert;
HitTestRequest::HitTestRequestType hit_type = HitTestRequest::kRelease;
HitTestRequest request(hit_type);
MouseEventWithHitTestResults mev = GetMouseEventTarget(request, mouse_event);
LocalFrame* subframe = event_handling_util::GetTargetSubframe(
Expand Down Expand Up @@ -1258,8 +1253,7 @@ WebInputEventResult EventHandler::UpdateDragAndDrop(
if (!frame_->View())
return event_result;

HitTestRequest request(HitTestRequest::kReadOnly |
HitTestRequest::kRetargetForInert);
HitTestRequest request(HitTestRequest::kReadOnly);
MouseEventWithHitTestResults mev =
event_handling_util::PerformMouseEventHitTest(frame_, request, event);

Expand Down Expand Up @@ -1945,7 +1939,6 @@ GestureEventWithHitTestResults EventHandler::HitTestResultForGestureEvent(
// disabled). Note that we don't yet apply hover/active state here because
// we need to resolve touch adjustment first so that we apply hover/active
// it to the final adjusted node.
hit_type |= HitTestRequest::kRetargetForInert;
hit_type |= HitTestRequest::kReadOnly;
WebGestureEvent adjusted_event = gesture_event;
LayoutSize hit_rect_size;
Expand Down Expand Up @@ -2062,8 +2055,7 @@ WebInputEventResult EventHandler::SendContextMenuEvent(

PhysicalOffset position_in_contents(
v->ConvertFromRootFrame(FlooredIntPoint(event.PositionInRootFrame())));
HitTestRequest request(HitTestRequest::kActive |
HitTestRequest::kRetargetForInert);
HitTestRequest request(HitTestRequest::kActive);
MouseEventWithHitTestResults mev =
frame_->GetDocument()->PerformMouseEventHitTest(
request, position_in_contents, event);
Expand Down Expand Up @@ -2185,8 +2177,7 @@ WebInputEventResult EventHandler::ShowNonLocatedContextMenu(
.origin();

// Use the focused node as the target for hover and active.
HitTestRequest request(HitTestRequest::kActive |
HitTestRequest::kRetargetForInert);
HitTestRequest request(HitTestRequest::kActive);
HitTestLocation location(location_in_root_frame);
HitTestResult result(request, location);
result.SetInnerNode(focused_element ? static_cast<Node*>(focused_element)
Expand Down Expand Up @@ -2272,8 +2263,7 @@ void EventHandler::HoverTimerFired(TimerBase*) {

if (auto* layout_object = frame_->ContentLayoutObject()) {
if (LocalFrameView* view = frame_->View()) {
HitTestRequest request(HitTestRequest::kMove |
HitTestRequest::kRetargetForInert);
HitTestRequest request(HitTestRequest::kMove);
HitTestLocation location(view->ViewportToFrame(
mouse_event_manager_->LastKnownMousePositionInViewport()));
HitTestResult result(request, location);
Expand All @@ -2291,8 +2281,7 @@ void EventHandler::ActiveIntervalTimerFired(TimerBase*) {
// FIXME: Enable condition when http://crbug.com/226842 lands
// m_lastDeferredTapElement.get() == m_frame->document()->activeElement()
HitTestRequest request(HitTestRequest::kTouchEvent |
HitTestRequest::kRelease |
HitTestRequest::kRetargetForInert);
HitTestRequest::kRelease);
frame_->GetDocument()->UpdateHoverActiveState(
request.Active(), !request.Move(), last_deferred_tap_element_.Get());
}
Expand Down Expand Up @@ -2325,8 +2314,7 @@ void EventHandler::DragSourceEndedAt(
ui::mojom::blink::DragOperation operation) {
// Asides from routing the event to the correct frame, the hit test is also an
// opportunity for Layer to update the :hover and :active pseudoclasses.
HitTestRequest request(HitTestRequest::kRelease |
HitTestRequest::kRetargetForInert);
HitTestRequest request(HitTestRequest::kRelease);
MouseEventWithHitTestResults mev =
event_handling_util::PerformMouseEventHitTest(frame_, request, event);

Expand Down
5 changes: 2 additions & 3 deletions blink/renderer/core/input/event_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,8 @@ class CORE_EXPORT EventHandler final : public GarbageCollected<EventHandler> {

HitTestResult HitTestResultAtLocation(
const HitTestLocation&,
HitTestRequest::HitTestRequestType hit_type =
HitTestRequest::kReadOnly | HitTestRequest::kActive |
HitTestRequest::kRetargetForInert,
HitTestRequest::HitTestRequestType hit_type = HitTestRequest::kReadOnly |
HitTestRequest::kActive,
const LayoutObject* stop_node = nullptr,
bool no_lifecycle_update = false);

Expand Down
3 changes: 1 addition & 2 deletions blink/renderer/core/input/gesture_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,7 @@ void GestureManager::Trace(Visitor* visitor) const {

HitTestRequest::HitTestRequestType GestureManager::GetHitTypeForGestureType(
WebInputEvent::Type type) {
HitTestRequest::HitTestRequestType hit_type =
HitTestRequest::kTouchEvent | HitTestRequest::kRetargetForInert;
HitTestRequest::HitTestRequestType hit_type = HitTestRequest::kTouchEvent;
switch (type) {
case WebInputEvent::Type::kGestureShowPress:
case WebInputEvent::Type::kGestureTapUnconfirmed:
Expand Down
3 changes: 1 addition & 2 deletions blink/renderer/core/input/mouse_event_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -894,8 +894,7 @@ bool MouseEventManager::HandleDrag(const MouseEventWithHitTestResults& event,
return false;

if (mouse_down_may_start_drag_) {
HitTestRequest request(HitTestRequest::kReadOnly |
HitTestRequest::kRetargetForInert);
HitTestRequest request(HitTestRequest::kReadOnly);
HitTestLocation location(mouse_down_pos_);
HitTestResult result(request, location);
frame_->ContentLayoutObject()->HitTest(location, result);
Expand Down
3 changes: 1 addition & 2 deletions blink/renderer/core/input/mouse_wheel_event_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,7 @@ Node* MouseWheelEventManager::FindTargetNode(const WebMouseWheelEvent& event,
PhysicalOffset v_point(
view->ConvertFromRootFrame(FlooredIntPoint(event.PositionInRootFrame())));

HitTestRequest request(HitTestRequest::kReadOnly |
HitTestRequest::kRetargetForInert);
HitTestRequest request(HitTestRequest::kReadOnly);
HitTestLocation location(v_point);
HitTestResult result(request, location);
doc->GetLayoutView()->HitTest(location, result);
Expand Down
12 changes: 5 additions & 7 deletions blink/renderer/core/input/pointer_event_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -364,8 +364,7 @@ void PointerEventManager::AdjustTouchPointerEvent(

HitTestRequest::HitTestRequestType hit_type =
HitTestRequest::kTouchEvent | HitTestRequest::kReadOnly |
HitTestRequest::kActive | HitTestRequest::kListBased |
HitTestRequest::kRetargetForInert;
HitTestRequest::kActive | HitTestRequest::kListBased;
LocalFrame& root_frame = frame_->LocalFrameRoot();
// TODO(szager): Shouldn't this be PositionInScreen() ?
PhysicalOffset hit_test_point = PhysicalOffset::FromFloatPointRound(
Expand Down Expand Up @@ -422,9 +421,9 @@ PointerEventManager::ComputePointerEventTarget(
// before firing the event.
if (web_pointer_event.GetType() == WebInputEvent::Type::kPointerDown ||
!pending_pointer_capture_target_.Contains(pointer_id)) {
HitTestRequest::HitTestRequestType hit_type =
HitTestRequest::kTouchEvent | HitTestRequest::kReadOnly |
HitTestRequest::kActive | HitTestRequest::kRetargetForInert;
HitTestRequest::HitTestRequestType hit_type = HitTestRequest::kTouchEvent |
HitTestRequest::kReadOnly |
HitTestRequest::kActive;
HitTestLocation location(frame_->View()->ConvertFromRootFrame(
PhysicalOffset::FromFloatPointRound(
FloatPoint(web_pointer_event.PositionInWidget()))));
Expand Down Expand Up @@ -861,8 +860,7 @@ WebInputEventResult PointerEventManager::SendMousePointerEvent(
// test to find the new target.
if (pointer_capture_target_.find(pointer_event->pointerId()) !=
pointer_capture_target_.end()) {
HitTestRequest::HitTestRequestType hit_type =
HitTestRequest::kRelease | HitTestRequest::kRetargetForInert;
HitTestRequest::HitTestRequestType hit_type = HitTestRequest::kRelease;
HitTestRequest request(hit_type);
MouseEventWithHitTestResults mev =
event_handling_util::PerformMouseEventHitTest(frame_, request,
Expand Down
3 changes: 1 addition & 2 deletions blink/renderer/core/input/scroll_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1051,8 +1051,7 @@ WebInputEventResult ScrollManager::HandleGestureScrollEvent(
LocalFrameView* view = frame_->View();
PhysicalOffset view_point(view->ConvertFromRootFrame(
FlooredIntPoint(gesture_event.PositionInRootFrame())));
HitTestRequest request(HitTestRequest::kReadOnly |
HitTestRequest::kRetargetForInert);
HitTestRequest request(HitTestRequest::kReadOnly);
HitTestLocation location(view_point);
HitTestResult result(request, location);
document->GetLayoutView()->HitTest(location, result);
Expand Down
12 changes: 5 additions & 7 deletions blink/renderer/core/layout/hit_test_request.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,15 @@ class HitTestRequest {
kAllowChildFrameContent = 1 << 8,
kChildFrameHitTest = 1 << 9,
kIgnorePointerEventsNone = 1 << 10,
kRetargetForInert = 1 << 11,
// Collect a list of nodes instead of just one.
// (This is for elementsFromPoint and rect-based tests).
kListBased = 1 << 12,
kListBased = 1 << 11,
// When using list-based testing, this flag causes us to continue hit
// testing after a hit has been found.
kPenetratingList = 1 << 13,
kAvoidCache = 1 << 14,
kIgnoreZeroOpacityObjects = 1 << 15,
kHitTestVisualOverflow = 1 << 16,
kPenetratingList = 1 << 12,
kAvoidCache = 1 << 13,
kIgnoreZeroOpacityObjects = 1 << 14,
kHitTestVisualOverflow = 1 << 15,
};

typedef unsigned HitTestRequestType;
Expand Down Expand Up @@ -87,7 +86,6 @@ class HitTestRequest {
bool IgnorePointerEventsNone() const {
return request_type_ & kIgnorePointerEventsNone;
}
bool RetargetForInert() const { return request_type_ & kRetargetForInert; }
bool ListBased() const { return request_type_ & kListBased; }
bool PenetratingList() const { return request_type_ & kPenetratingList; }
bool AvoidCache() const { return request_type_ & kAvoidCache; }
Expand Down
38 changes: 1 addition & 37 deletions blink/renderer/core/layout/hit_test_result.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ HitTestResult::HitTestResult(const HitTestResult& other)
: hit_test_request_(other.hit_test_request_),
cacheable_(other.cacheable_),
inner_node_(other.InnerNode()),
inert_node_(other.InertNode()),
inner_element_(other.InnerElement()),
inner_possibly_pseudo_node_(other.inner_possibly_pseudo_node_),
point_in_inner_node_frame_(other.point_in_inner_node_frame_),
Expand All @@ -98,7 +97,7 @@ HitTestResult& HitTestResult::operator=(const HitTestResult& other) {

bool HitTestResult::EqualForCacheability(const HitTestResult& other) const {
return hit_test_request_.EqualForCacheability(other.hit_test_request_) &&
inner_node_ == other.InnerNode() && inert_node_ == other.InertNode() &&
inner_node_ == other.InnerNode() &&
inner_element_ == other.InnerElement() &&
inner_possibly_pseudo_node_ == other.InnerPossiblyPseudoNode() &&
point_in_inner_node_frame_ == other.point_in_inner_node_frame_ &&
Expand All @@ -115,7 +114,6 @@ void HitTestResult::CacheValues(const HitTestResult& other) {

void HitTestResult::PopulateFromCachedResult(const HitTestResult& other) {
inner_node_ = other.InnerNode();
inert_node_ = other.InertNode();
inner_element_ = other.InnerElement();
inner_possibly_pseudo_node_ = other.InnerPossiblyPseudoNode();
point_in_inner_node_frame_ = other.point_in_inner_node_frame_;
Expand All @@ -137,7 +135,6 @@ void HitTestResult::PopulateFromCachedResult(const HitTestResult& other) {
void HitTestResult::Trace(Visitor* visitor) const {
visitor->Trace(hit_test_request_);
visitor->Trace(inner_node_);
visitor->Trace(inert_node_);
visitor->Trace(inner_element_);
visitor->Trace(inner_possibly_pseudo_node_);
visitor->Trace(inner_url_element_);
Expand Down Expand Up @@ -303,24 +300,6 @@ void HitTestResult::SetInnerNode(Node* n) {
return;
}

if (RuntimeEnabledFeatures::InertAttributeEnabled()) {
// TODO(crbug.com/692360): Remove inert retargeting, and instead use
// 'pointer-events: none'.
if (GetHitTestRequest().RetargetForInert()) {
if (n->IsInert() && n != n->GetDocument().documentElement()) {
if (!inert_node_)
inert_node_ = n;

return;
}

if (inert_node_ && n != inert_node_ &&
!n->IsShadowIncludingInclusiveAncestorOf(*inert_node_)) {
return;
}
}
}

inner_possibly_pseudo_node_ = n;
if (auto* pseudo_element = DynamicTo<PseudoElement>(n))
n = pseudo_element->InnerNodeForHitTesting();
Expand All @@ -335,14 +314,6 @@ void HitTestResult::SetInnerNode(Node* n) {
inner_element_ = FlatTreeTraversal::ParentElement(*inner_node_);
}

void HitTestResult::SetInertNode(Node* n) {
// Don't overwrite an existing value for inert_node_
if (inert_node_)
DCHECK(n == inert_node_);

inert_node_ = n;
}

void HitTestResult::SetURLElement(Element* n) {
inner_url_element_ = n;
}
Expand Down Expand Up @@ -520,10 +491,6 @@ std::tuple<bool, ListBasedHitTestBehavior>
HitTestResult::AddNodeToListBasedTestResultInternal(
Node* node,
const HitTestLocation& location) {
// If we are in the process of retargeting for `inert`, continue.
if (GetHitTestRequest().RetargetForInert() && InertNode() && !InnerNode())
return std::make_tuple(false, kContinueHitTesting);

// If not a list-based test, stop testing because the hit has been found.
if (!GetHitTestRequest().ListBased())
return std::make_tuple(false, kStopHitTesting);
Expand Down Expand Up @@ -601,9 +568,6 @@ void HitTestResult::Append(const HitTestResult& other) {
canvas_region_id_ = other.CanvasRegionId();
}

if (!inert_node_ && other.InertNode())
SetInertNode(other.InertNode());

if (other.list_based_test_result_) {
NodeSet& set = MutableListBasedTestResult();
for (NodeSet::const_iterator it = other.list_based_test_result_->begin(),
Expand Down
3 changes: 0 additions & 3 deletions blink/renderer/core/layout/hit_test_result.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ class CORE_EXPORT HitTestResult {
// FIXME: Make these less error-prone for rect-based hit tests (center point
// or fail).
Node* InnerNode() const { return inner_node_.Get(); }
Node* InertNode() const { return inert_node_.Get(); }
Node* InnerPossiblyPseudoNode() const {
return inner_possibly_pseudo_node_.Get();
}
Expand Down Expand Up @@ -141,7 +140,6 @@ class CORE_EXPORT HitTestResult {
const HitTestRequest& GetHitTestRequest() const { return hit_test_request_; }

void SetInnerNode(Node*);
void SetInertNode(Node*);
HTMLAreaElement* ImageAreaForImage() const;
void SetURLElement(Element*);
void SetScrollbar(Scrollbar*);
Expand Down Expand Up @@ -213,7 +211,6 @@ class CORE_EXPORT HitTestResult {
bool cacheable_;

Member<Node> inner_node_;
Member<Node> inert_node_;
// This gets calculated in the first call to InnerElement function.
Member<Element> inner_element_;
Member<Node> inner_possibly_pseudo_node_;
Expand Down
1 change: 0 additions & 1 deletion blink/renderer/core/layout/layout_embedded_content.cc
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,6 @@ bool LayoutEmbeddedContent::NodeAtPoint(
result.GetHitTestRequest().GetStopNode());
HitTestResult child_frame_result(new_hit_test_request,
new_hit_test_location);
child_frame_result.SetInertNode(result.InertNode());

// The frame's layout and style must be up to date if we reach here.
bool is_inside_child_frame = child_layout_view->HitTestNoLifecycleUpdate(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,6 @@ bool LayoutSVGForeignObject::NodeAtPointFromSVG(
// element, but PaintLayer::HitTestLayer assumes it has not been.
HitTestLocation local_without_offset(*local_location, -PhysicalLocation());
HitTestResult layer_result(result.GetHitTestRequest(), local_without_offset);
layer_result.SetInertNode(result.InertNode());
bool retval = Layer()->HitTest(local_without_offset, layer_result,
PhysicalRect(PhysicalRect::InfiniteIntRect()));

Expand Down
Loading

0 comments on commit 440e5c9

Please sign in to comment.