For loop not working as expected with string replacement

Question

I am trying to make a javascript webextension that adds a couple of numbers eg. "123" to the end of the inner text of a hyperlink text to each product on a shopping website, eg. http://www.tomleemusic.ca

For example, if I go to this link, http://tomleemusic.ca/catalogsearch/result/?cat=0&q=piano

I want to add the item's identification number to the end of the product's name.

name of product and the href tag are in its item link, tomleemusic.ca/xxxxxx with the x's being the item number

However with my following code, I simply append the item number of the first item in the list to every item, instead of a different item number for each item.

var productsListLink = document.querySelectorAll(".products-grid .item .product-name a:not(.product-image)");
for (var i = 0; i < productsListLink.length; i++) {
    var a = productsListLink[i];
    var name = a.innerHTML || "";
    var addon = document.querySelector(".products-grid .item .product-name a:not(.product-image)").getAttribute('href');
    var newaddon = addon.replace("http://tomleemusic.ca/","");
    name += newaddon;
    a.innerHTML = name;
    a.setAttribute('title', name);
}

score 3 · Answer 1 · answered Sep 08 '18 at 07:42

In this line, you're grabbing only the first matching element:

var addon = document.querySelector(".products-grid .item .product-name a:not(.product-image)").getAttribute('href')

You already have the element you're actually working with in each loop iteration in a; just use that instead:

var addon = a.getAttribute('href')

Example:

var productsListLink = document.querySelectorAll(".products-grid .item .product-name a:not(.product-image)");
for (var i = 0; i < productsListLink.length; i++) {
    var a = productsListLink[i];
    var name = a.innerHTML || "";
    var addon = a.getAttribute('href');
    var newaddon = addon.replace("http://tomleemusic.ca/","");
    name += newaddon;
    a.innerHTML = name;
    a.setAttribute('title', name);
}

<div class="products-grid">
  <div class="item">
    <span class="product-name">
      <a href="http://tomleemusic.ca/1"></a>
    </span>
  </div>
  <div class="item">
    <span class="product-name">
      <a href="http://tomleemusic.ca/2"></a>
    </span>
  </div>
  <div class="item">
    <span class="product-name">
      <a href="http://tomleemusic.ca/3"></a>
    </span>
  </div>
</div>

Thank you! this was very helpful – George Xu Sep 08 '18 at 16:01 — George Xu, Sep 08 '18 at 16:01

CertainPerformance · Accepted Answer · 2018-09-08T07:47:12.710

2

querySelector will always return the first matching element. Thus, when you do

var addon = document.querySelector(".products-grid .item .product-name a:not(.product-image)").getAttribute('href');

you're selecting the first a (the one you get in your first iteration).

But, you can make the code a whole lot cleaner by using array methods and a regular expression to match the id:

Array.prototype.forEach.call(
  document.querySelectorAll(".products-grid .item .product-name a:not(.product-image)"),
  (productNameElement) => {
    const idMatch = productNameElement.href.match(/\d+$/);
    if (idMatch) productNameElement.appendChild(document.createTextNode(idMatch[0]));
  });

Also note that only some of the elements have an ID number. For example, one of the search results:

<a href="http://tomleemusic.ca/benchworld-sonata-1c-single-adjustable-artist-piano-bench-in-polished-walnut" title="BENCHWORLD SONATA 1c Single Adjustable Artist Piano Bench In Polished Walnut">BENCHWORLD SONATA 1c Single Adjustable Artist <span class="searchindex-highlight">Piano</span> Bench In Polished Walnut</a>

doesn't have one, so it would be good to check that there's a match first.

edited Sep 08 '18 at 07:47

answered Sep 08 '18 at 07:44

CertainPerformance

356,069
52
309
320

Note that `NodeList#forEach` may still need polyfilling on slightly-older in-use browsers; [here's an answer of mine with a polyfill](https://stackoverflow.com/questions/46929157/foreach-on-queryselectorall-not-working-in-recent-microsoft-browsers/46929259#46929259). – T.J. Crowder Sep 08 '18 at 07:46
I didn't have the impression from the question that the `replace` part wasn't working as expected...? – T.J. Crowder Sep 08 '18 at 07:47
If OP is looking for the *identification number*, a few of the products on the page don't have any, so the output might not be as expected when those pop up, hence the regex. Not entirely sure what's desired when that happens – CertainPerformance Sep 08 '18 at 07:52
Yeah, it's not clear, but your approach of defending against it makes sense. – T.J. Crowder Sep 08 '18 at 08:50
Thank you for the detailed answer! I only started java a couple months ago so I 'm still working on cleaning my code up :) this was very hlepful – George Xu Sep 08 '18 at 16:02
@GeorgeXu - It's JavaScript, not Java. Java is an entirely different language. :-) – T.J. Crowder Sep 08 '18 at 17:12
@T.J.Crowder oops brainfart, I started java a couple months ago and javascript only for this project – George Xu Sep 10 '18 at 07:27

For loop not working as expected with string replacement

2 Answers2