merge again

content/base/public/nsINode.h

@@ -1078,6 +1078,12 @@ public:
   nsresult QuerySelector(const nsAString& aSelector, nsIDOMElement **aReturn);
   nsresult QuerySelectorAll(const nsAString& aSelector, nsIDOMNodeList **aReturn);
 
+protected:
+  // nsIDocument overrides this with its own (faster) version.  This
+  // should really only be called for elements and document fragments.
+  mozilla::dom::Element* GetElementById(const nsAString& aId);
+
+public:
   /**
    * Associate an object aData to aKey on this node. If aData is null any
    * previously registered object and UserDataHandler associated to aKey on

content/base/src/DocumentFragment.h

@@ -40,6 +40,8 @@ public:
   using FragmentOrElement::GetFirstChild;
   using nsINode::QuerySelector;
   using nsINode::QuerySelectorAll;
+  // Make sure bindings can see our superclass' protected GetElementById method.
+  using nsINode::GetElementById;
 
   // nsISupports
   NS_DECL_ISUPPORTS_INHERITED

content/base/src/nsINode.cpp

@@ -2319,6 +2319,59 @@ AddScopeElements(TreeMatchContext& aMatchContext,
   }
 }
 
+namespace {
+struct SelectorMatchInfo {
+  nsCSSSelectorList* const mSelectorList;
+  TreeMatchContext& mMatchContext;
+};
+}
+
+// Given an id, find elements with that id under aRoot that match aMatchInfo if
+// any is provided.  If no SelectorMatchInfo is provided, just find the ones
+// with the given id.  aRoot must be in the document.
+template<bool onlyFirstMatch, class T>
+inline static void
+FindMatchingElementsWithId(const nsAString& aId, nsINode* aRoot,
+                           SelectorMatchInfo* aMatchInfo,
+                           T& aList)
+{
+  MOZ_ASSERT(aRoot->IsInDoc(),
+             "Don't call me if the root is not in the document");
+  MOZ_ASSERT(aRoot->IsElement() || aRoot->IsNodeOfType(nsINode::eDOCUMENT),
+             "The optimization below to check ContentIsDescendantOf only for "
+             "elements depends on aRoot being either an element or a "
+             "document if it's in the document.  Note that document fragments "
+             "can't be IsInDoc(), so should never show up here.");
+
+  const nsSmallVoidArray* elements = aRoot->OwnerDoc()->GetAllElementsForId(aId);
+
+  if (!elements) {
+    // Nothing to do; we're done
+    return;
+  }
+
+  // XXXbz: Should we fall back to the tree walk if aRoot is not the
+  // document and |elements| is long, for some value of "long"?
+  for (int32_t i = 0; i < elements->Count(); ++i) {
+    Element *element = static_cast<Element*>(elements->ElementAt(i));
+    if (!aRoot->IsElement() ||
+        (element != aRoot &&
+           nsContentUtils::ContentIsDescendantOf(element, aRoot))) {
+      // We have an element with the right id and it's a strict descendant
+      // of aRoot.  Make sure it really matches the selector.
+      if (!aMatchInfo ||
+          nsCSSRuleProcessor::SelectorListMatches(element,
+                                                  aMatchInfo->mMatchContext,
+                                                  aMatchInfo->mSelectorList)) {
+        aList.AppendElement(element);
+        if (onlyFirstMatch) {
+          return;
+        }
+      }
+    }
+  }
+}
+
 // Actually find elements matching aSelectorList (which must not be
 // null) and which are descendants of aRoot and put them in aList.  If
 // onlyFirstMatch, then stop once the first one is found.
@@ -2326,7 +2379,6 @@ template<bool onlyFirstMatch, class Collector, class T>
 MOZ_ALWAYS_INLINE static nsresult
 FindMatchingElements(nsINode* aRoot, const nsAString& aSelector, T &aList)
 {
-
   nsIDocument* doc = aRoot->OwnerDoc();
   nsIDocument::SelectorCache& cache = doc->GetSelectorCache();
   nsCSSSelectorList* selectorList = nullptr;
@@ -2380,32 +2432,9 @@ FindMatchingElements(nsINode* aRoot, const nsAString& aSelector, T &aList)
       !selectorList->mNext &&
       selectorList->mSelectors->mIDList) {
     nsIAtom* id = selectorList->mSelectors->mIDList->mAtom;
-    const nsSmallVoidArray* elements =
+    SelectorMatchInfo info = { selectorList, matchingContext };
-      doc->GetAllElementsForId(nsDependentAtomString(id));
+    FindMatchingElementsWithId<onlyFirstMatch, T>(nsDependentAtomString(id),
-
+                                                  aRoot, &info, aList);
-    // XXXbz: Should we fall back to the tree walk if aRoot is not the
-    // document and |elements| is long, for some value of "long"?
-    if (elements) {
-      for (int32_t i = 0; i < elements->Count(); ++i) {
-        Element *element = static_cast<Element*>(elements->ElementAt(i));
-        if (!aRoot->IsElement() ||
-            (element != aRoot &&
-             nsContentUtils::ContentIsDescendantOf(element, aRoot))) {
-          // We have an element with the right id and it's a strict descendant
-          // of aRoot.  Make sure it really matches the selector.
-          if (nsCSSRuleProcessor::SelectorListMatches(element, matchingContext,
-                                                      selectorList)) {
-            aList.AppendElement(element);
-            if (onlyFirstMatch) {
-              return NS_OK;
-            }
-          }
-        }
-      }
-    }
-
-    // No elements with this id, or none of them are our descendants,
-    // or none of them match.  We're done here.
     return NS_OK;
   }
 
@@ -2492,6 +2521,29 @@ nsINode::QuerySelectorAll(const nsAString& aSelector, nsIDOMNodeList **aReturn)
   return rv.ErrorCode();
 }
 
+Element*
+nsINode::GetElementById(const nsAString& aId)
+{
+  MOZ_ASSERT(IsElement() || IsNodeOfType(eDOCUMENT_FRAGMENT),
+             "Bogus this object for GetElementById call");
+  if (IsInDoc()) {
+    ElementHolder holder;
+    FindMatchingElementsWithId<true>(aId, this, nullptr, holder);
+    return holder.mElement;
+  }
+
+  for (nsIContent* kid = GetFirstChild(); kid; kid = kid->GetNextNode(this)) {
+    if (!kid->IsElement()) {
+      continue;
+    }
+    nsIAtom* id = kid->AsElement()->GetID();
+    if (id && id->Equals(aId)) {
+      return kid->AsElement();
+    }
+  }
+  return nullptr;
+}
+
 JSObject*
 nsINode::WrapObject(JSContext *aCx, JS::Handle<JSObject*> aScope)
 {

content/base/test/mochitest.ini

@@ -532,6 +532,7 @@ support-files =
 [test_elementTraversal.html]
 [test_fileapi.html]
 [test_fileapi_slice.html]
+[test_getElementById.html]
 [test_html_colors_quirks.html]
 [test_html_colors_standards.html]
 [test_html_in_xhr.html]

content/base/test/test_getElementById.html

@@ -0,0 +1,58 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=933193
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 933193</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=933193">Mozilla Bug 933193</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+</pre>
+  <script type="application/javascript">
+
+  /** Test for Bug 933193 **/
+    var kid = document.createElement("span");
+    kid.id = "test";
+    var svg = document.createElementNS("http://www.w3.org/2000/svg", "svg");
+    svg.appendChild(kid);
+    is(svg.getElementById("test"), kid,
+       "Should find the right node when not in the DOM");
+
+    var newKid = document.createElement("span");
+    newKid.id = "test";
+    var newKidParent = document.createElement("span");
+    newKidParent.appendChild(newKid);
+    svg.insertBefore(newKidParent, kid);
+    is(svg.getElementById("test"), newKid,
+       "Should find the first right node when not in the DOM");
+    newKid.remove();
+    is(svg.getElementById("test"), kid,
+       "Should find the right node again when not in the DOM");
+
+    document.body.appendChild(svg);
+    is(svg.getElementById("test"), kid,
+       "Should find the right node when in the DOM");
+
+    is(document.getElementById("test").localName, "pre",
+       "document.getElementById should find the first element in the " +
+       "document with that id");
+
+    var frag = document.createDocumentFragment();
+    is(frag.getElementById("test"), null, "Shouldn't find what does not exist");
+    frag.appendChild(kid);
+    is(frag.getElementById("test"), kid,
+       "Should find the right node in the document fragment");
+    is(svg.getElementById("test"), null,
+       "Shouldn't find the kid since it's gone now");
+  </script>
+</body>
+</html>

content/svg/content/src/SVGSVGElement.cpp

@@ -435,18 +435,6 @@ SVGSVGElement::CreateSVGTransformFromMatrix(SVGMatrix& matrix)
   return transform.forget();
 }
 
-Element*
-SVGSVGElement::GetElementById(const nsAString& elementId, ErrorResult& rv)
-{
-  nsAutoString selector(NS_LITERAL_STRING("#"));
-  nsStyleUtil::AppendEscapedCSSIdent(PromiseFlatString(elementId), selector);
-  nsIContent* element = QuerySelector(selector, rv);
-  if (!rv.Failed() && element) {
-    return element->AsElement();
-  }
-  return nullptr;
-}
-
 //----------------------------------------------------------------------
 
 already_AddRefed<SVGAnimatedRect>

content/svg/content/src/SVGSVGElement.h

@@ -246,7 +246,7 @@ public:
   already_AddRefed<SVGIRect> CreateSVGRect();
   already_AddRefed<SVGTransform> CreateSVGTransform();
   already_AddRefed<SVGTransform> CreateSVGTransformFromMatrix(SVGMatrix& matrix);
-  Element* GetElementById(const nsAString& elementId, ErrorResult& rv);
+  using nsINode::GetElementById; // This does what we want
   already_AddRefed<SVGAnimatedRect> ViewBox();
   already_AddRefed<DOMSVGAnimatedPreserveAspectRatio> PreserveAspectRatio();
   uint16_t ZoomAndPan();

dom/webidl/DocumentFragment.webidl

@@ -13,13 +13,7 @@
 
 [Constructor]
 interface DocumentFragment : Node {
-  // NEW
+  Element? getElementById(DOMString elementId);
-  /*
-    FIXME: not implemented yet
-
-  void prepend((Node or DOMString)... nodes);
-  void append((Node or DOMString)... nodes);
-  */
 };
 
 // http://www.w3.org/TR/2012/WD-selectors-api-20120628/#interface-definitions

dom/webidl/SVGSVGElement.webidl

@@ -59,7 +59,6 @@ interface SVGSVGElement : SVGGraphicsElement {
   SVGTransform createSVGTransform();
   [NewObject]
   SVGTransform createSVGTransformFromMatrix(SVGMatrix matrix);
-  [Throws]
   Element? getElementById(DOMString elementId);
 };