third_party/WebKit/Source/core/editing/SelectionController.cpp - Issue 2537243002: Adding |Node::layoutObject()| null check to |setCaretAtHitTestResult()|

Unified Diff: third_party/WebKit/Source/core/editing/SelectionController.cpp

Issue 2537243002: Adding |Node::layoutObject()| null check to |setCaretAtHitTestResult()| (Closed)

Patch Set: Created 4 years, 1 month ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Index: third_party/WebKit/Source/core/editing/SelectionController.cpp

diff --git a/third_party/WebKit/Source/core/editing/SelectionController.cpp b/third_party/WebKit/Source/core/editing/SelectionController.cpp

index ef28dcf60498451a8d8fe22ea842de0e731d32df..f0efc8647377abcc6146ddeec405df94300d33db 100644

--- a/third_party/WebKit/Source/core/editing/SelectionController.cpp

+++ b/third_party/WebKit/Source/core/editing/SelectionController.cpp

@@ -652,7 +652,7 @@ void SelectionController::setNonDirectionalSelectionIfNeeded(

bool SelectionController::setCaretAtHitTestResult(

const HitTestResult& hitTestResult) {

Node* innerNode = hitTestResult.innerNode();

- if (!innerNode)

+ if (!innerNode || !innerNode->layoutObject())

yosin_UTC9 2016/11/30 02:01:48 It is strange that HitTestResult picks up "display

amaralp 2016/12/01 01:25:34 I didn't think it was possible for the HitTestResu

yosin_UTC9 2016/12/01 01:51:09 We need to find where we call HitTestResult::setIn

We need to find where we call HitTestResult::setInnerNode() with node->layoutObject() == nullptr. Then we can assemble such HTML and simulate events in gTest. Below is short investigation: Insane HitTestReulst is calculated in GestureManager::handleGestureLongPress() with IntPoint hitTestPoint = m_frame->view()->rootFrameToContents(gestureEvent.position()); HitTestResult hitTestResult = m_frame->eventHandler().hitTestResultAtPoint(hitTestPoint); Expect for <body style="display:none">, all touch event should have visible node. So, suspicious is rootFrameToContents() returns unexpected point then hitTestResultAtPoint() returns insane HitTestResult. In EventHandler::hitTestResultAtPoint(), following code fragment can return insane HitTestResult: // LayoutView::hitTest causes a layout, and we don't want to hit that until // the first layout because until then, there is nothing shown on the screen - // the user can't have intentionally clicked on something belonging to this // page. Furthermore, mousemove events before the first layout should not // lead to a premature layout() happening, which could show a flash of white. // See also the similar code in Document::performMouseEventHitTest. if (m_frame->contentLayoutItem().isNull() || !m_frame->view() || !m_frame->view()->didFirstLayout()) return result; **** innerNode == nullptr m_frame->contentLayoutItem().hitTest(result); But this isn't innerNode()->layoutObject() == nullptr case. Fumm...

return false;

const VisiblePositionInFlatTree& visibleHitPos =

« no previous file with comments | « no previous file | no next file » | no next file with comments »