Changeset 154945 in webkit

Timestamp:

Sep 1, 2013 12:12:31 PM (11 years ago)

Author:

Darin Adler

Message:

HitTestResult should have innerNonSharedElement
​https://bugs.webkit.org/show_bug.cgi?id=120579

Reviewed by Andreas Kling.

• editing/Editor.cpp:

(WebCore::Editor::copyImage): Call HitTestResult member function version of
innerNonSharedElement instead of a local function that does it.

• page/Chrome.cpp:

(WebCore::Chrome::setToolTip): Use innerNonSharedElement instead of getting
the node and checking if it's an input element. Also added some missing braces.

• page/EventHandler.cpp:

(WebCore::EventHandler::selectClosestWordFromHitTestResult): Use targetNode for
local variables instead of innerNode to match the HitTestResult function name.
(WebCore::EventHandler::selectClosestWordOrLinkFromMouseEvent): Ditto.
(WebCore::EventHandler::handleMousePressEventTripleClick): Ditto.
(WebCore::EventHandler::handleMousePressEventSingleClick): Ditto.
(WebCore::EventHandler::handleMousePressEvent): Ditto.

• rendering/HitTestResult.cpp:

(WebCore::HitTestResult::innerElement): Rewrote so there there is no loop.
(WebCore::HitTestResult::innerNonSharedElement): Ditto.

• rendering/HitTestResult.h: Added innerNonSharedElement. Generally speaking,

we'd like to avoid using Node unless there is some real need.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• editing/Editor.cpp (modified) (1 diff)
• page/Chrome.cpp (modified) (3 diffs)
• page/EventHandler.cpp (modified) (10 diffs)
• rendering/HitTestResult.cpp (modified) (1 diff)
• rendering/HitTestResult.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-09-01  Darin Adler  <darin@apple.com>
+
+        HitTestResult should have innerNonSharedElement
+        https://bugs.webkit.org/show_bug.cgi?id=120579
+
+        Reviewed by Andreas Kling.
+
+        * editing/Editor.cpp:
+        (WebCore::Editor::copyImage): Call HitTestResult member function version of
+        innerNonSharedElement instead of a local function that does it.
+
+        * page/Chrome.cpp:
+        (WebCore::Chrome::setToolTip): Use innerNonSharedElement instead of getting
+        the node and checking if it's an input element. Also added some missing braces.
+
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::selectClosestWordFromHitTestResult): Use targetNode for
+        local variables instead of innerNode to match the HitTestResult function name.
+        (WebCore::EventHandler::selectClosestWordOrLinkFromMouseEvent): Ditto.
+        (WebCore::EventHandler::handleMousePressEventTripleClick): Ditto.
+        (WebCore::EventHandler::handleMousePressEventSingleClick): Ditto.
+        (WebCore::EventHandler::handleMousePressEvent): Ditto.
+
+        * rendering/HitTestResult.cpp:
+        (WebCore::HitTestResult::innerElement): Rewrote so there there is no loop.
+        (WebCore::HitTestResult::innerNonSharedElement): Ditto.
+
+        * rendering/HitTestResult.h: Added innerNonSharedElement. Generally speaking,
+        we'd like to avoid using Node unless there is some real need.
+
 2013-09-01  Xabier Rodriguez Calvar  <calvaris@igalia.com>
 

trunk/Source/WebCore/editing/Editor.cpp

@@ -1172 +1172 @@
 }
 
-// FIXME: Should this be a member function of HitTestResult?
-static Element* innerNonSharedElement(const HitTestResult& result)
-{
-    Node* node = result.innerNonSharedNode();
-    if (!node)
-        return 0;
-    if (node->isElementNode())
-        return toElement(node);
-    return node->parentElement();
-}
-
 void Editor::copyImage(const HitTestResult& result)
 {
-    Element* element = innerNonSharedElement(result);
+    Element* element = result.innerNonSharedElement();
     if (!element)
         return;

trunk/Source/WebCore/page/Chrome.cpp

@@ -403 +403 @@
     // Next priority is a toolTip from a URL beneath the mouse (if preference is set to show those).
     if (toolTip.isEmpty() && m_page->settings().showsURLsInToolTips()) {
-        if (Node* node = result.innerNonSharedNode()) {
+        if (Element* element = result.innerNonSharedElement()) {
             // Get tooltip representing form action, if relevant
-            if (isHTMLInputElement(node)) {
-                HTMLInputElement* input = toHTMLInputElement(node);
-                if (input->isSubmitButton())
+            if (isHTMLInputElement(element)) {
+                HTMLInputElement* input = toHTMLInputElement(element);
+                if (input->isSubmitButton()) {
                     if (HTMLFormElement* form = input->form()) {
                         toolTip = form->action();
@@ -415 +415 @@
                             toolTipDirection = LTR;
                     }
+                }
             }
         }
@@ -436 +437 @@
     // Lastly, for <input type="file"> that allow multiple files, we'll consider a tooltip for the selected filenames
     if (toolTip.isEmpty()) {
-        if (Node* node = result.innerNonSharedNode()) {
-            if (isHTMLInputElement(node)) {
-                HTMLInputElement* input = toHTMLInputElement(node);
-                toolTip = input->defaultToolTip();
+        if (Element* element = result.innerNonSharedElement()) {
+            if (isHTMLInputElement(element)) {
+                toolTip = toHTMLInputElement(element)->defaultToolTip();
 
                 // FIXME: We should obtain text direction of tooltip from

trunk/Source/WebCore/page/EventHandler.cpp

@@ -486 +486 @@
 void EventHandler::selectClosestWordFromHitTestResult(const HitTestResult& result, AppendTrailingWhitespace appendTrailingWhitespace)
 {
-    Node* innerNode = result.targetNode();
+    Node* targetNode = result.targetNode();
     VisibleSelection newSelection;
 
-    if (innerNode && innerNode->renderer()) {
-        VisiblePosition pos(innerNode->renderer()->positionForPoint(result.localPoint()));
+    if (targetNode && targetNode->renderer()) {
+        VisiblePosition pos(targetNode->renderer()->positionForPoint(result.localPoint()));
         if (pos.isNotNull()) {
             newSelection = VisibleSelection(pos);
@@ -499 +499 @@
             newSelection.appendTrailingWhitespace();
 
-        updateSelectionForMouseDownDispatchingSelectStart(innerNode, expandSelectionToRespectUserSelectAll(innerNode, newSelection), WordGranularity);
+        updateSelectionForMouseDownDispatchingSelectStart(targetNode, expandSelectionToRespectUserSelectAll(targetNode, newSelection), WordGranularity);
     }
 }
@@ -516 +516 @@
         return selectClosestWordFromMouseEvent(result);
 
-    Node* innerNode = result.targetNode();
-
-    if (innerNode && innerNode->renderer() && m_mouseDownMayStartSelect) {
+    Node* targetNode = result.targetNode();
+
+    if (targetNode && targetNode->renderer() && m_mouseDownMayStartSelect) {
         VisibleSelection newSelection;
         Element* URLElement = result.hitTestResult().URLElement();
-        VisiblePosition pos(innerNode->renderer()->positionForPoint(result.localPoint()));
+        VisiblePosition pos(targetNode->renderer()->positionForPoint(result.localPoint()));
         if (pos.isNotNull() && pos.deepEquivalent().deprecatedNode()->isDescendantOf(URLElement))
             newSelection = VisibleSelection::selectionFromContentsOfNode(URLElement);
 
-        updateSelectionForMouseDownDispatchingSelectStart(innerNode, expandSelectionToRespectUserSelectAll(innerNode, newSelection), WordGranularity);
+        updateSelectionForMouseDownDispatchingSelectStart(targetNode, expandSelectionToRespectUserSelectAll(targetNode, newSelection), WordGranularity);
     }
 }
@@ -552 +552 @@
         return false;
     
-    Node* innerNode = event.targetNode();
-    if (!(innerNode && innerNode->renderer() && m_mouseDownMayStartSelect))
+    Node* targetNode = event.targetNode();
+    if (!(targetNode && targetNode->renderer() && m_mouseDownMayStartSelect))
         return false;
 
     VisibleSelection newSelection;
-    VisiblePosition pos(innerNode->renderer()->positionForPoint(event.localPoint()));
+    VisiblePosition pos(targetNode->renderer()->positionForPoint(event.localPoint()));
     if (pos.isNotNull()) {
         newSelection = VisibleSelection(pos);
@@ -563 +563 @@
     }
 
-    return updateSelectionForMouseDownDispatchingSelectStart(innerNode, expandSelectionToRespectUserSelectAll(innerNode, newSelection), ParagraphGranularity);
+    return updateSelectionForMouseDownDispatchingSelectStart(targetNode, expandSelectionToRespectUserSelectAll(targetNode, newSelection), ParagraphGranularity);
 }
 
@@ -575 +575 @@
 {
     m_frame->document()->updateLayoutIgnorePendingStylesheets();
-    Node* innerNode = event.targetNode();
-    if (!(innerNode && innerNode->renderer() && m_mouseDownMayStartSelect))
+    Node* targetNode = event.targetNode();
+    if (!(targetNode && targetNode->renderer() && m_mouseDownMayStartSelect))
         return false;
 
@@ -592 +592 @@
     }
 
-    VisiblePosition visiblePos(innerNode->renderer()->positionForPoint(event.localPoint()));
+    VisiblePosition visiblePos(targetNode->renderer()->positionForPoint(event.localPoint()));
     if (visiblePos.isNull())
-        visiblePos = VisiblePosition(firstPositionInOrBeforeNode(innerNode), DOWNSTREAM);
+        visiblePos = VisiblePosition(firstPositionInOrBeforeNode(targetNode), DOWNSTREAM);
     Position pos = visiblePos.deepEquivalent();
 
@@ -601 +601 @@
 
     if (extendSelection && newSelection.isCaretOrRange()) {
-        VisibleSelection selectionInUserSelectAll = expandSelectionToRespectUserSelectAll(innerNode, VisibleSelection(pos));
+        VisibleSelection selectionInUserSelectAll = expandSelectionToRespectUserSelectAll(targetNode, VisibleSelection(pos));
         if (selectionInUserSelectAll.isRange()) {
             if (comparePositions(selectionInUserSelectAll.start(), newSelection.start()) < 0)
@@ -628 +628 @@
         }
     } else
-        newSelection = expandSelectionToRespectUserSelectAll(innerNode, visiblePos);
-
-    bool handled = updateSelectionForMouseDownDispatchingSelectStart(innerNode, newSelection, granularity);
+        newSelection = expandSelectionToRespectUserSelectAll(targetNode, visiblePos);
+
+    bool handled = updateSelectionForMouseDownDispatchingSelectStart(targetNode, newSelection, granularity);
 
     if (event.event().button() == MiddleButton) {
@@ -697 +697 @@
         focusDocumentView();
 
-    Node* innerNode = event.targetNode();
-
-    m_mousePressNode = innerNode;
+    m_mousePressNode = event.targetNode();
 #if ENABLE(DRAG_SUPPORT)
     m_dragStartPos = event.event().position();

trunk/Source/WebCore/rendering/HitTestResult.cpp

@@ -691 +691 @@
 Element* HitTestResult::innerElement() const
 {
-    for (Node* node = m_innerNode.get(); node; node = node->parentNode()) {
-        if (node->isElementNode())
-            return toElement(node);
-    }
-
-    return 0;
+    Node* node = m_innerNode.get();
+    if (!node)
+        return 0;
+    if (node->isElementNode())
+        return toElement(node);
+    return node->parentElement();
+}
+
+Element* HitTestResult::innerNonSharedElement() const
+{
+    Node* node = m_innerNonSharedNode.get();
+    if (!node)
+        return 0;
+    if (node->isElementNode())
+        return toElement(node);
+    return node->parentElement();
 }
 

trunk/Source/WebCore/rendering/HitTestResult.h

@@ -63 +63 @@
     Element* innerElement() const;
     Node* innerNonSharedNode() const { return m_innerNonSharedNode.get(); }
+    Element* innerNonSharedElement() const;
     Element* URLElement() const { return m_innerURLElement.get(); }
     Scrollbar* scrollbar() const { return m_scrollbar.get(); }