Remove closestAncestorElementByTag

Just use `closestAncestorElementBySelector`
ampproject · Feb 21, 2019 · efa27d0 · efa27d0
1 parent 2dc43d8
commit efa27d0
Show file tree

Hide file tree

Showing 18 changed files with 37 additions and 54 deletions.
diff --git a/build-system/conformance-config.textproto b/build-system/conformance-config.textproto
@@ -19,7 +19,7 @@ requirement: {
 
 requirement: {
   type: BANNED_PROPERTY_CALL
-  error_message: 'Use closestAncestorElementBySelector, closestAncestorElementByTag etc in src/dom.js'
+  error_message: 'Use closestAncestorElementBySelector in src/dom.js'
   value: 'Element.prototype.closest'
   whitelist: 'src/dom.js'
 }

diff --git a/extensions/amp-ad/0.1/amp-ad-custom.js b/extensions/amp-ad/0.1/amp-ad-custom.js
@@ -21,7 +21,7 @@ import {Services} from '../../../src/services';
 import {addParamToUrl} from '../../../src/url';
 import {
   childElementByTag,
-  closestAncestorElementByTag,
+  closestAncestorElementBySelector,
   removeChildren,
 } from '../../../src/dom';
 import {hasOwn} from '../../../src/utils/object';
@@ -211,7 +211,7 @@ export class AmpAdCustom extends AMP.BaseElement {
 
       // Get the parent body of this amp-ad element. It could be the body of
       // the main document, or it could be an enclosing iframe.
-      const body = closestAncestorElementByTag(this.element, 'BODY');
+      const body = closestAncestorElementBySelector(this.element, 'BODY');
       const elements = body.querySelectorAll('amp-ad[type=custom]');
       for (let index = 0; index < elements.length; index++) {
         const elem = elements[index];

diff --git a/extensions/amp-audio/0.1/amp-audio.js b/extensions/amp-audio/0.1/amp-audio.js
@@ -23,7 +23,7 @@ import {
 } from '../../../src/mediasession-helper';
 import {Layout} from '../../../src/layout';
 import {assertHttpsUrl} from '../../../src/url';
-import {closestAncestorElementByTag} from '../../../src/dom';
+import {closestAncestorElementBySelector} from '../../../src/dom';
 import {dev, user} from '../../../src/log';
 import {getMode} from '../../../src/mode';
 import {listen} from '../../../src/event-helper';
@@ -209,7 +209,7 @@ export class AmpAudio extends AMP.BaseElement {
    * @private
    */
   isStoryDescendant_() {
-    return closestAncestorElementByTag(this.element, 'AMP-STORY');
+    return closestAncestorElementBySelector(this.element, 'AMP-STORY');
   }
 
   /** @private */

diff --git a/extensions/amp-auto-ads/0.1/placement.js b/extensions/amp-auto-ads/0.1/placement.js
@@ -21,7 +21,7 @@ import {
 import {Services} from '../../../src/services';
 import {clamp} from '../../../src/utils/math';
 import {
-  closestAncestorElementByTag,
+  closestAncestorElementBySelector,
   createElementWithAttributes,
   scopedQuerySelectorAll,
   whenUpgradedToCustomElement,
@@ -425,7 +425,7 @@ function isPositionValid(anchorElement, position) {
   }
   const elementToCheck = dev().assertElement(elementToCheckOrNull);
   return !BLACKLISTED_ANCESTOR_TAGS.some(tagName => {
-    if (closestAncestorElementByTag(elementToCheck, tagName)) {
+    if (closestAncestorElementBySelector(elementToCheck, tagName)) {
       user().warn(TAG, 'Placement inside blacklisted ancestor: ' + tagName);
       return true;
     }

diff --git a/extensions/amp-bind/0.1/bind-impl.js b/extensions/amp-bind/0.1/bind-impl.js
@@ -21,7 +21,7 @@ import {ChunkPriority, chunk} from '../../../src/chunk';
 import {RAW_OBJECT_ARGS_KEY} from '../../../src/action-constants';
 import {Services} from '../../../src/services';
 import {Signals} from '../../../src/utils/signals';
-import {closestAncestorElementByTag, iterateCursor} from '../../../src/dom';
+import {closestAncestorElementBySelector, iterateCursor} from '../../../src/dom';
 import {debounce} from '../../../src/utils/rate-limit';
 import {deepEquals, getValueForExpr, parseJson} from '../../../src/json';
 import {deepMerge, dict, map} from '../../../src/utils/object';
@@ -1198,7 +1198,7 @@ export class Bind {
     if (!Services.platformFor(this.win_).isSafari()) {
       return;
     }
-    const select = closestAncestorElementByTag(element, 'select');
+    const select = closestAncestorElementBySelector(element, 'select');
     if (!select) {
       return;
     }

diff --git a/extensions/amp-date-picker/0.1/amp-date-picker.js b/extensions/amp-date-picker/0.1/amp-date-picker.js
@@ -26,7 +26,7 @@ import {Layout, isLayoutSizeDefined} from '../../../src/layout';
 import {Services} from '../../../src/services';
 import {batchFetchJsonFor} from '../../../src/batched-json';
 import {
-  closestAncestorElementByTag,
+  closestAncestorElementBySelector,
   escapeCssSelectorIdent,
   isRTL,
   iterateCursor,
@@ -832,7 +832,7 @@ export class AmpDatePicker extends AMP.BaseElement {
       return existingField;
     }
 
-    const form = closestAncestorElementByTag(this.element, 'form');
+    const form = closestAncestorElementBySelector(this.element, 'form');
     if (this.mode_ == DatePickerMode.STATIC && form) {
       const hiddenInput = this.document_.createElement('input');
       hiddenInput.type = 'hidden';

diff --git a/extensions/amp-image-lightbox/0.1/amp-image-lightbox.js b/extensions/amp-image-lightbox/0.1/amp-image-lightbox.js
@@ -917,7 +917,7 @@ class AmpImageLightbox extends AMP.BaseElement {
     let caption = null;
 
     // 1. Check <figure> and <figcaption>.
-    const figure = dom.closestAncestorElementByTag(sourceElement, 'figure');
+    const figure = dom.closestAncestorElementBySelector(sourceElement, 'figure');
     if (figure) {
       caption = dom.elementByTag(figure, 'figcaption');
     }

diff --git a/extensions/amp-install-serviceworker/0.1/amp-install-serviceworker.js b/extensions/amp-install-serviceworker/0.1/amp-install-serviceworker.js
@@ -15,7 +15,7 @@
  */
 
 import {Services} from '../../../src/services';
-import {closestAncestorElementByTag, removeElement} from '../../../src/dom';
+import {closestAncestorElementBySelector, removeElement} from '../../../src/dom';
 import {dev, user, userAssert} from '../../../src/log';
 import {dict} from '../../../src/utils/object';
 import {getMode} from '../../../src/mode';
@@ -268,7 +268,7 @@ class UrlRewriter_ {
       return;
     }
     const target =
-      closestAncestorElementByTag(dev().assertElement(event.target), 'A');
+      closestAncestorElementBySelector(dev().assertElement(event.target), 'A');
     if (!target || !target.href) {
       return;
     }

diff --git a/extensions/amp-lightbox-gallery/0.1/service/lightbox-manager-impl.js b/extensions/amp-lightbox-gallery/0.1/service/lightbox-manager-impl.js
@@ -26,7 +26,7 @@ import {Services} from '../../../../src/services';
 import {
   childElement,
   childElementByAttr,
-  closestAncestorElementByTag,
+  closestAncestorElementBySelector,
   elementByTag,
   iterateCursor,
 } from '../../../../src/dom';
@@ -303,7 +303,7 @@ export class LightboxManager {
   getDescription(element) {
     // If the element in question is the descendant of a figure element
     // try using the figure caption as the lightbox description.
-    const figureParent = closestAncestorElementByTag(element, 'figure');
+    const figureParent = closestAncestorElementBySelector(element, 'figure');
     if (figureParent) {
       const figCaption = elementByTag(figureParent, 'figcaption');
       if (figCaption) {

diff --git a/extensions/amp-sidebar/0.1/amp-sidebar.js b/extensions/amp-sidebar/0.1/amp-sidebar.js
@@ -19,7 +19,7 @@ import {CSS} from '../../../build/amp-sidebar-0.1.css';
 import {Keys} from '../../../src/utils/key-codes';
 import {Services} from '../../../src/services';
 import {Toolbar} from './toolbar';
-import {closestAncestorElementByTag, isRTL, tryFocus} from '../../../src/dom';
+import {closestAncestorElementBySelector, isRTL, tryFocus} from '../../../src/dom';
 import {createCustomEvent} from '../../../src/event-helper';
 import {descendsFromStory} from '../../../src/utils/story';
 import {dev} from '../../../src/log';
@@ -186,7 +186,7 @@ export class AmpSidebar extends AMP.BaseElement {
 
     element.addEventListener('click', e => {
       const target =
-        closestAncestorElementByTag(dev().assertElement(e.target), 'A');
+        closestAncestorElementBySelector(dev().assertElement(e.target), 'A');
       if (target && target.href) {
         const tgtLoc = Services.urlForDoc(element).parse(target.href);
         const currentHref = this.getAmpDoc().win.location.href;

diff --git a/extensions/amp-story/0.1/amp-story-consent.js b/extensions/amp-story/0.1/amp-story-consent.js
@@ -23,7 +23,7 @@ import {Services} from '../../../src/services';
 import {assertAbsoluteHttpOrHttpsUrl} from '../../../src/url';
 import {
   childElementByTag,
-  closestAncestorElementByTag,
+  closestAncestorElementBySelector,
   isJsonScriptTag,
 } from '../../../src/dom';
 import {computedStyle, setImportantStyles} from '../../../src/style';
@@ -197,8 +197,8 @@ export class AmpStoryConsent extends AMP.BaseElement {
   buildCallback() {
     this.assertAndParseConfig_();
 
-    const storyEl = closestAncestorElementByTag(this.element, 'AMP-STORY');
-    const consentEl = closestAncestorElementByTag(this.element, 'AMP-CONSENT');
+    const storyEl = closestAncestorElementBySelector(this.element, 'AMP-STORY');
+    const consentEl = closestAncestorElementBySelector(this.element, 'AMP-CONSENT');
     const consentId = consentEl.id;
     this.storeService_.dispatch(Action.SET_CONSENT_ID, consentId);
 

diff --git a/extensions/amp-story/1.0/amp-story-access.js b/extensions/amp-story/1.0/amp-story-access.js
@@ -23,7 +23,7 @@ import {Layout} from '../../../src/layout';
 import {assertHttpsUrl} from '../../../src/url';
 import {
   closest,
-  closestAncestorElementByTag,
+  closestAncestorElementBySelector,
   copyChildren,
   removeChildren,
 } from '../../../src/dom';
@@ -243,7 +243,7 @@ export class AmpStoryAccess extends AMP.BaseElement {
    */
   getLogoSrc_() {
     const storyEl = dev().assertElement(
-        closestAncestorElementByTag(this.element, 'AMP-STORY'));
+        closestAncestorElementBySelector(this.element, 'AMP-STORY'));
     const logoSrc = storyEl && storyEl.getAttribute('publisher-logo-src');
 
     logoSrc ?

diff --git a/extensions/amp-story/1.0/amp-story-consent.js b/extensions/amp-story/1.0/amp-story-consent.js
@@ -27,7 +27,7 @@ import {Services} from '../../../src/services';
 import {assertAbsoluteHttpOrHttpsUrl, assertHttpsUrl} from '../../../src/url';
 import {
   childElementByTag,
-  closestAncestorElementByTag,
+  closestAncestorElementBySelector,
   isJsonScriptTag,
 } from '../../../src/dom';
 import {computedStyle, setImportantStyles} from '../../../src/style';
@@ -201,8 +201,8 @@ export class AmpStoryConsent extends AMP.BaseElement {
     this.assertAndParseConfig_();
 
     const storyEl = dev().assertElement(
-        closestAncestorElementByTag(this.element, 'AMP-STORY'));
-    const consentEl = closestAncestorElementByTag(this.element, 'AMP-CONSENT');
+        closestAncestorElementBySelector(this.element, 'AMP-STORY'));
+    const consentEl = closestAncestorElementBySelector(this.element, 'AMP-CONSENT');
     const consentId = consentEl.id;
 
     this.storeConsentId_(consentId);

diff --git a/src/custom-element.js b/src/custom-element.js
@@ -745,7 +745,7 @@ function createBaseCustomElementClass(win) {
     connectedCallback() {
       if (!isTemplateTagSupported() && this.isInTemplate_ === undefined) {
         this.isInTemplate_ =
-          !!dom.closestAncestorElementByTag(this, 'template');
+          !!dom.closestAncestorElementBySelector(this, 'template');
       }
       if (this.isInTemplate_) {
         return;

diff --git a/src/dom.js b/src/dom.js
@@ -259,23 +259,6 @@ export function closestNode(node, callback) {
 }
 
 
-/**
- * Finds the closest ancestor element with the specified name from this element
- * up the DOM subtree.
- * @param {!Element} element
- * @param {string} tagName
- * @return {?Element}
- */
-export function closestAncestorElementByTag(element, tagName) {
-  if (element.closest) {
-    return element.closest(tagName);
-  }
-  tagName = tagName.toUpperCase();
-  return closest(element, el => {
-    return el.tagName == tagName;
-  });
-}
-
 /**
  * Finds the closest ancestor element with the specified selector from this
  * element.

diff --git a/src/service/navigation.js b/src/service/navigation.js
@@ -16,7 +16,7 @@
 
 import {Services} from '../services';
 import {
-  closestAncestorElementByTag,
+  closestAncestorElementBySelector,
   escapeCssSelectorIdent,
   isIframed,
   openWindowDialog,
@@ -311,7 +311,7 @@ export class Navigation {
       return;
     }
     const element = dev().assertElement(e.target);
-    const target = closestAncestorElementByTag(element, 'A');
+    const target = closestAncestorElementBySelector(element, 'A');
     if (!target || !target.href) {
       return;
     }
@@ -601,7 +601,7 @@ export class Navigation {
  */
 function maybeExpandUrlParams(ampdoc, e) {
   const target =
-    closestAncestorElementByTag(dev().assertElement(e.target), 'A');
+    closestAncestorElementBySelector(dev().assertElement(e.target), 'A');
   if (!target || !target.href) {
     // Not a click on a link.
     return;

diff --git a/src/utils/story.js b/src/utils/story.js
@@ -14,7 +14,7 @@
   * limitations under the License.
   */
 
-import {closestAncestorElementByTag, waitForChild} from '../dom';
+import {closestAncestorElementBySelector, waitForChild} from '../dom';
 
 
 /**
@@ -26,7 +26,7 @@ import {closestAncestorElementByTag, waitForChild} from '../dom';
  * @return {boolean}
  */
 export function descendsFromStory(element) {
-  return !!closestAncestorElementByTag(element, 'amp-story');
+  return !!closestAncestorElementBySelector(element, 'amp-story');
 }
 
 /**

diff --git a/test/unit/test-dom.js b/test/unit/test-dom.js
@@ -187,8 +187,8 @@ describes.sandboxed('DOM', {}, env => {
 
     expect(dom.closest(child, () => true)).to.equal(child);
     expect(dom.closestNode(child, () => true)).to.equal(child);
-    expect(dom.closestAncestorElementByTag(child, 'div')).to.equal(child);
-    expect(dom.closestAncestorElementByTag(child, 'DIV')).to.equal(child);
+    expect(dom.closestAncestorElementBySelector(child, 'div')).to.equal(child);
+    expect(dom.closestAncestorElementBySelector(child, 'DIV')).to.equal(child);
   });
 
   it('closest should stop search at opt_stopAt', () => {
@@ -223,16 +223,16 @@ describes.sandboxed('DOM', {}, env => {
 
     expect(dom.closest(child, e => e.tagName == 'CHILD')).to.equal(child);
     expect(dom.closestNode(child, e => e.tagName == 'CHILD')).to.equal(child);
-    expect(dom.closestAncestorElementByTag(child, 'child')).to.equal(child);
+    expect(dom.closestAncestorElementBySelector(child, 'child')).to.equal(child);
 
     expect(dom.closest(child, e => e.tagName == 'ELEMENT')).to.equal(element);
     expect(dom.closestNode(child, e => e.tagName == 'ELEMENT'))
         .to.equal(element);
-    expect(dom.closestAncestorElementByTag(child, 'element')).to.equal(element);
+    expect(dom.closestAncestorElementBySelector(child, 'element')).to.equal(element);
 
     expect(dom.closest(child, e => e.tagName == 'PARENT')).to.equal(parent);
     expect(dom.closestNode(child, e => e.tagName == 'PARENT')).to.equal(parent);
-    expect(dom.closestAncestorElementByTag(child, 'parent')).to.equal(parent);
+    expect(dom.closestAncestorElementBySelector(child, 'parent')).to.equal(parent);
   });
 
   it('closestNode should find nodes as well as elements', () => {