From c0bf67d1f97bca3dbb696580fd51ce0730f52d9b Mon Sep 17 00:00:00 2001 From: Elliott Sprehn Date: Fri, 31 Oct 2014 18:48:50 -0700 Subject: [PATCH] Remove interactive content and form related API from HTMLElement. In particular this inlines the accessKeyAction, removes isInteractiveContent() which was for special casing certain elements in event dispatch, and removes eventParameterName() which was only needed for inline event handlers. I also moved Node::commonAncestor into NodeRenderingTraversal, the only callers of it passed NodeRenderingTraversal::parent as the second argument, it didn't make sense to have this generic thing. R=abarth@chromium.org, ojan@chromium.org Review URL: https://codereview.chromium.org/693243002 --- engine/core/dom/Element.h | 2 -- engine/core/dom/Node.cpp | 37 ---------------------- engine/core/dom/Node.h | 1 - engine/core/dom/NodeRenderingTraversal.cpp | 37 ++++++++++++++++++++++ engine/core/dom/NodeRenderingTraversal.h | 1 + engine/core/html/HTMLAnchorElement.cpp | 5 --- engine/core/html/HTMLAnchorElement.h | 1 - engine/core/html/HTMLElement.cpp | 16 ---------- engine/core/html/HTMLElement.h | 8 ----- engine/core/html/HTMLImageElement.cpp | 5 --- engine/core/html/HTMLImageElement.h | 1 - engine/core/html/HTMLMediaElement.cpp | 5 --- engine/core/html/HTMLMediaElement.h | 1 - engine/core/page/EventHandler.cpp | 17 +++------- 14 files changed, 43 insertions(+), 94 deletions(-) diff --git a/engine/core/dom/Element.h b/engine/core/dom/Element.h index fbb7df73a2e..b4ea12891ae 100644 --- a/engine/core/dom/Element.h +++ b/engine/core/dom/Element.h @@ -228,8 +228,6 @@ public: AtomicString computeInheritedLanguage() const; Locale& locale() const; - virtual void accessKeyAction(bool /*sendToAnyEvent*/) { } - virtual bool isURLAttribute(const Attribute&) const { return false; } virtual bool isLiveLink() const { return false; } diff --git a/engine/core/dom/Node.cpp b/engine/core/dom/Node.cpp index bf54bb6e240..c56762732f3 100644 --- a/engine/core/dom/Node.cpp +++ b/engine/core/dom/Node.cpp @@ -752,43 +752,6 @@ bool Node::containsIncludingHostElements(const Node& node) const return false; } -Node* Node::commonAncestor(const Node& other, Node* (*parent)(const Node&)) -{ - if (this == other) - return this; - if (document() != other.document()) - return 0; - int thisDepth = 0; - for (Node* node = this; node; node = parent(*node)) { - if (node == &other) - return node; - thisDepth++; - } - int otherDepth = 0; - for (const Node* node = &other; node; node = parent(*node)) { - if (node == this) - return this; - otherDepth++; - } - Node* thisIterator = this; - const Node* otherIterator = &other; - if (thisDepth > otherDepth) { - for (int i = thisDepth; i > otherDepth; --i) - thisIterator = parent(*thisIterator); - } else if (otherDepth > thisDepth) { - for (int i = otherDepth; i > thisDepth; --i) - otherIterator = parent(*otherIterator); - } - while (thisIterator) { - if (thisIterator == otherIterator) - return thisIterator; - thisIterator = parent(*thisIterator); - otherIterator = parent(*otherIterator); - } - ASSERT(!otherIterator); - return 0; -} - void Node::reattach(const AttachContext& context) { AttachContext reattachContext(context); diff --git a/engine/core/dom/Node.h b/engine/core/dom/Node.h index a642dd3ee8d..77e115e212f 100644 --- a/engine/core/dom/Node.h +++ b/engine/core/dom/Node.h @@ -404,7 +404,6 @@ public: bool contains(const Node*) const; bool containsIncludingShadowDOM(const Node*) const; bool containsIncludingHostElements(const Node&) const; - Node* commonAncestor(const Node&, Node* (*parent)(const Node&)); // Used to determine whether range offsets use characters or node indices. virtual bool offsetInCharacters() const; diff --git a/engine/core/dom/NodeRenderingTraversal.cpp b/engine/core/dom/NodeRenderingTraversal.cpp index 1b2fe5ff4c0..b1dda328f31 100644 --- a/engine/core/dom/NodeRenderingTraversal.cpp +++ b/engine/core/dom/NodeRenderingTraversal.cpp @@ -137,6 +137,43 @@ RenderObject* previousSiblingRenderer(const Node* node) return 0; } +Node* commonAncestor(Node& a, Node& b) +{ + if (a == b) + return &a; + if (a.document() != b.document()) + return 0; + int thisDepth = 0; + for (Node* node = &a; node; node = parent(node)) { + if (node == b) + return node; + thisDepth++; + } + int otherDepth = 0; + for (const Node* node = &b; node; node = parent(node)) { + if (node == a) + return &a; + otherDepth++; + } + Node* thisIterator = &a; + const Node* otherIterator = &b; + if (thisDepth > otherDepth) { + for (int i = thisDepth; i > otherDepth; --i) + thisIterator = parent(thisIterator); + } else if (otherDepth > thisDepth) { + for (int i = otherDepth; i > thisDepth; --i) + otherIterator = parent(otherIterator); + } + while (thisIterator) { + if (thisIterator == otherIterator) + return thisIterator; + thisIterator = parent(thisIterator); + otherIterator = parent(otherIterator); + } + ASSERT(!otherIterator); + return 0; +} + } } // namespace diff --git a/engine/core/dom/NodeRenderingTraversal.h b/engine/core/dom/NodeRenderingTraversal.h index 57dbee2ad75..564d8feec95 100644 --- a/engine/core/dom/NodeRenderingTraversal.h +++ b/engine/core/dom/NodeRenderingTraversal.h @@ -61,6 +61,7 @@ Node* nextSibling(const Node*); Node* previousSibling(const Node*); Node* previous(const Node*, const Node* stayWithin); Node* next(const Node*, const Node* stayWithin); +Node* commonAncestor(Node&, Node&); RenderObject* nextSiblingRenderer(const Node*); RenderObject* previousSiblingRenderer(const Node*); diff --git a/engine/core/html/HTMLAnchorElement.cpp b/engine/core/html/HTMLAnchorElement.cpp index 73ac2a64643..8a08fec2e33 100644 --- a/engine/core/html/HTMLAnchorElement.cpp +++ b/engine/core/html/HTMLAnchorElement.cpp @@ -148,9 +148,4 @@ bool HTMLAnchorElement::willRespondToMouseClickEvents() return isLiveLink(); } -bool HTMLAnchorElement::isInteractiveContent() const -{ - return isLiveLink(); -} - } diff --git a/engine/core/html/HTMLAnchorElement.h b/engine/core/html/HTMLAnchorElement.h index 7c2a9a91ef0..0eaf76f8ca1 100644 --- a/engine/core/html/HTMLAnchorElement.h +++ b/engine/core/html/HTMLAnchorElement.h @@ -52,7 +52,6 @@ private: virtual bool isURLAttribute(const Attribute&) const override final; virtual bool hasLegalLinkAttribute(const QualifiedName&) const override final; virtual bool canStartSelection() const override final; - virtual bool isInteractiveContent() const override final; void handleClick(Event*); }; diff --git a/engine/core/html/HTMLElement.cpp b/engine/core/html/HTMLElement.cpp index 7d1775513da..64d29e0e175 100644 --- a/engine/core/html/HTMLElement.cpp +++ b/engine/core/html/HTMLElement.cpp @@ -84,11 +84,6 @@ void HTMLElement::click() dispatchSimulatedClick(0, SendNoEvents); } -void HTMLElement::accessKeyAction(bool sendMouseEvents) -{ - dispatchSimulatedClick(0, sendMouseEvents ? SendMouseUpDownEvents : SendNoEvents); -} - String HTMLElement::title() const { return getAttribute(HTMLNames::titleAttr); @@ -130,17 +125,6 @@ void HTMLElement::setDir(const AtomicString& value) setAttribute(HTMLNames::dirAttr, value); } -bool HTMLElement::isInteractiveContent() const -{ - return false; -} - -const AtomicString& HTMLElement::eventParameterName() -{ - DEFINE_STATIC_LOCAL(const AtomicString, eventString, ("event", AtomicString::ConstructFromLiteral)); - return eventString; -} - v8::Handle HTMLElement::wrap(v8::Handle creationContext, v8::Isolate* isolate) { return createV8HTMLWrapper(this, creationContext, isolate); diff --git a/engine/core/html/HTMLElement.h b/engine/core/html/HTMLElement.h index 09675709f16..d4a6d49f123 100644 --- a/engine/core/html/HTMLElement.h +++ b/engine/core/html/HTMLElement.h @@ -50,14 +50,6 @@ public: void click(); - virtual void accessKeyAction(bool sendMouseEvents) override; - - virtual bool isLabelable() const { return false; } - // http://www.whatwg.org/specs/web-apps/current-work/multipage/elements.html#interactive-content - virtual bool isInteractiveContent() const; - - static const AtomicString& eventParameterName(); - virtual v8::Handle wrap(v8::Handle creationContext, v8::Isolate*) override; protected: diff --git a/engine/core/html/HTMLImageElement.cpp b/engine/core/html/HTMLImageElement.cpp index ef1534f332a..af8c527bd82 100644 --- a/engine/core/html/HTMLImageElement.cpp +++ b/engine/core/html/HTMLImageElement.cpp @@ -464,11 +464,6 @@ Image* HTMLImageElement::imageContents() return imageLoader().image()->image(); } -bool HTMLImageElement::isInteractiveContent() const -{ - return hasAttribute(HTMLNames::usemapAttr); -} - PassRefPtr HTMLImageElement::getSourceImageForCanvas(SourceImageMode, SourceImageStatus* status) const { if (!complete() || !cachedImage()) { diff --git a/engine/core/html/HTMLImageElement.h b/engine/core/html/HTMLImageElement.h index 82ce6dfb007..6e9c0a1e2d5 100644 --- a/engine/core/html/HTMLImageElement.h +++ b/engine/core/html/HTMLImageElement.h @@ -111,7 +111,6 @@ private: virtual InsertionNotificationRequest insertedInto(ContainerNode*) override; virtual void removedFrom(ContainerNode*) override; - virtual bool isInteractiveContent() const override; virtual Image* imageContents() override; ImageCandidate findBestFitImageFromPictureParent(); diff --git a/engine/core/html/HTMLMediaElement.cpp b/engine/core/html/HTMLMediaElement.cpp index 4b613205ea5..5205593c074 100644 --- a/engine/core/html/HTMLMediaElement.cpp +++ b/engine/core/html/HTMLMediaElement.cpp @@ -2414,11 +2414,6 @@ void HTMLMediaElement::mediaPlayerMediaSourceOpened(blink::WebMediaSource* webMe m_mediaSource->setWebMediaSourceAndOpen(adoptPtr(webMediaSource)); } -bool HTMLMediaElement::isInteractiveContent() const -{ - return hasAttribute(HTMLNames::controlsAttr); -} - void HTMLMediaElement::defaultEventHandler(Event* event) { HTMLElement::defaultEventHandler(event); diff --git a/engine/core/html/HTMLMediaElement.h b/engine/core/html/HTMLMediaElement.h index c177069f856..f04f93d04f2 100644 --- a/engine/core/html/HTMLMediaElement.h +++ b/engine/core/html/HTMLMediaElement.h @@ -209,7 +209,6 @@ private: virtual void removedFrom(ContainerNode*) override final; virtual void didRecalcStyle(StyleRecalcChange) override final; - virtual bool isInteractiveContent() const override final; virtual void defaultEventHandler(Event*) override final; // ActiveDOMObject functions. diff --git a/engine/core/page/EventHandler.cpp b/engine/core/page/EventHandler.cpp index 715bc939f41..7ae123a095d 100644 --- a/engine/core/page/EventHandler.cpp +++ b/engine/core/page/EventHandler.cpp @@ -1246,15 +1246,6 @@ void EventHandler::invalidateClick() m_clickNode = nullptr; } -static Node* parentForClickEvent(const Node& node) -{ - // IE doesn't dispatch click events for mousedown/mouseup events across form - // controls. - if (node.isHTMLElement() && toHTMLElement(node).isInteractiveContent()) - return 0; - return NodeRenderingTraversal::parent(&node); -} - bool EventHandler::handleMouseReleaseEvent(const PlatformMouseEvent& mouseEvent) { TRACE_EVENT0("blink", "EventHandler::handleMouseReleaseEvent"); @@ -1293,7 +1284,7 @@ bool EventHandler::handleMouseReleaseEvent(const PlatformMouseEvent& mouseEvent) bool swallowClickEvent = false; if (m_clickCount > 0 && mev.targetNode() && m_clickNode) { - if (Node* clickTargetNode = mev.targetNode()->commonAncestor(*m_clickNode, parentForClickEvent)) + if (Node* clickTargetNode = NodeRenderingTraversal::commonAncestor(*mev.targetNode(), *m_clickNode)) swallowClickEvent = !dispatchMouseEvent(EventTypeNames::click, clickTargetNode, m_clickCount, mouseEvent, true); } @@ -1770,7 +1761,7 @@ bool EventHandler::handleGestureTap(const GestureEventWithHitTestResults& target bool swallowClickEvent = false; if (m_clickNode) { if (currentHitTest.innerNode()) { - Node* clickTargetNode = currentHitTest.innerNode()->commonAncestor(*m_clickNode, parentForClickEvent); + Node* clickTargetNode = NodeRenderingTraversal::commonAncestor(*currentHitTest.innerNode(), *m_clickNode); swallowClickEvent = !dispatchMouseEvent(EventTypeNames::click, clickTargetNode, gestureEvent.tapCount(), fakeMouseUp, true); } m_clickNode = nullptr; @@ -2270,7 +2261,9 @@ bool EventHandler::handleAccessKey(const PlatformKeyboardEvent& evt) Element* elem = m_frame->document()->getElementByAccessKey(key.lower()); if (!elem) return false; - elem->accessKeyAction(false); + // FIXME(sky): We should probably pass SendMouseUpDownEvents here, not + // firing mouseup and mousedown is IE legacy. + elem->dispatchSimulatedClick(0, SendNoEvents); return true; }