Ampersand in innerHtml() not being read correctly and validating in JS

Question

I just realized I screwed up the original HTML - the icon classes should not have been originally on the code. Now I understand the downvoting - along with the general sloppiness of the javascript. I've modified the HTML, making it what it should have been to start with.

I've got an unordered list that I'm searching through with jQuery to add a class to its anchor tag, depending on the text/innerHtml of the link that is in the list item:

<ul class="menu-items">
<li class="menu-item icon-globe">
    <a href="/test/Mobile/Accounts.aspx" class="ui-link">Accounts</a></li>
<li class="menu-item icon-globe">
    <a href="/dupont/Mobile/Transfers.aspx" class="ui-link">Transfers</a></li>
<li class="menu-item emailMenuItem icon-globe">
    <a href="/test/Mobile/Messages.aspx" class="ui-link">Messages</a></li>
<li class="menu-item">
    <a href="/test/CustomerService.aspx" class="ui-link">Alerts &amp; Settings</a></li>
<li class="menu-item emailMenuItem icon-globe"><a href="/test/Mobile/DepositCheck.aspx" class="ui-link">Deposit a Check</a>

Her is the jQuery statement that should be adding the "AlertSettingIcon" class to the Alerts & Settings anchor tag:

$("li.menu-item a.ui-link").each(function() {
    if ($( this ).innerHtml == "Accounts") {
        $(this).addClass("AccountIcon");
    } else
        if ($( this ).innerHtml == "Transfers") {
            $(this).addClass("TransferIcon");
        } else
            if ($( this ).innerHtml == "Messages") {
                $(this).addClass("MessageIcon");
            } else
                if ($( this ).innerHtml == "Alerts &amp; Settings") { 
                    $(this).addClass("AlertSettingIcon");
                } else
                    if ($( this ).innerHtml() == "Deposit a Check") {
                        $(this).addClass("RDCIcon");
                    }
                    else {
                        $(this).addClass("AlertSettingIcon");
                    }

});

When I use developer tools to examine the classes assigned to each anchor tag, the class does not get added. I've tried using .text(), wrapping my script in CDATA tags, .html(), using /u0026, and a couple of other methods, but I have yet to successfully add the class to the "Alerts & Settings" anchor element. I've also done each of these methods in both firefox and Chrome to ensure it's not browser compatibility.

Finally, here's a JSFiddle where the issue shows itself: https://jsfiddle.net/r7vnft5c/

Can anyone figure out why the AlertSettingIcon class is not being added as the JS/jQuery is written, and what I can do to fix it?

Thanks for your ideas, David

It is bad practice to use innerHTML in if/else statements like in your code. Also, I don't think 'innerHtml()' is correct syntax. — opznhaarlems, Mar 18 '15 at 15:14
Indeed, innerHTML is a string, not a function. Remove the (). But you should never compare something with innerHTML as different browsers can ad or remove spaces between elements to "beautify" the code, so the innerHTML value is unpredictable. — Domino, Mar 18 '15 at 15:21
@opznhaarlems, If you downvoted, I'd love to know why the innerHtml is bad practice in the if/then - and what the fix is. innerHtml() functions perfectly in 4 out of the 5 elements, and when using firebug to try and identify what value the JS is reading from the text of the Alerts & Settings link, it showed innerHtml as giving the version of "Alerts & Settings", which seemed to be a better option that using "Alerts & Settings",which JS may read incorrectly (at least from multiple other threads I've read trying to solve this problem. — David, Mar 18 '15 at 15:24
@David I must say, I'm not sure why you're doing this, what exactly is the use case for this code? It seems to me a strange bit of code, if I knew what it was for I could probably show you a better way of doing it. — Mike, Mar 18 '15 at 15:26
I didn't downvote you, but I'd imagine it's because your demo and jsFiddle are very sloppy. You've now removed some of the `()` (but not all), and it's still wrong because `$(this).innerHtml` should be `this.innerHTML` or `$(this).html()`. So the trouble is that you make a claim that the code works a certain way when the code you posted doesn't. — Lye Fish, Mar 18 '15 at 15:35
@Mike The use case is that this UL is rendered from an ASP.net back end that I don't have access to override, so by inserting the js on the master page, I can identify specific links and add css - what I'm not showing is that the CSS adds an icon to a jQuery mobile menu. The only unique identifier of each li a is it's text, so it's what I'm using to access the item to add the class — David, Mar 18 '15 at 15:47
Here's your original code with the following changes... ***1)*** Fixed `this.innerHTML` ***2)*** Actually loaded jQuery in the demo ***3)*** Added CSS rules to make the result immediately visible. ***DEMO:*** http://jsfiddle.net/r7vnft5c/2/ Works just fine with those changes. — Lye Fish, Mar 18 '15 at 15:52
...oh, and the reason it ***appeared*** to work up to a point with your original code was that ***you had the class names hardcoded in the HTML*** for the first three items. — Lye Fish, Mar 18 '15 at 15:54
@LyeFish, I realized that just about a minute ago (that my code had the class names on it) - - that really screwed me up in this post. Thanks for sticking with it and pointing it out. My goal is to utilize either the switch:case or the map that you added to clean it up. In any case, I'm going to mark your answer as correct. Thanks for sticking with it and helping me understand what was going on. — David, Mar 18 '15 at 15:58

Lye Fish · Accepted Answer · 2015-03-18T15:39:25.290

.innerHTML isn't a method. For jQuery, it's $(this).html() or with the native API, it would be this.innerHTML.

And use more sensible flow control constructs like a switch statement.

Generally, .innerHTML comparison can be touchy. Use .text() instead when you're only comparing the text content.

$("li.menu-item a.ui-link").each(function() {
    switch ($(this).text()) {
    case "Accounts":
        $(this).addClass("AccountIcon"); break;
    case "Transfers":
        $(this).addClass("TransferIcon"); break;
    case "Messages":
        $(this).addClass("MessageIcon"); break;
    case "Alerts & Settings":
         $(this).addClass("AlertSettingIcon"); break;
    case "Deposit a Check":
         $(this).addClass("RDCIcon"); break;
    default:
         $(this).addClass("AlertSettingIcon");
    }
});

Another way would be to have a map of the HTML to the class name...

var map = {
  Accounts: "AccountIcon",
  Transfers: "TransferIcon",
  Messages: "MessageIcon",
  "Alerts & Settings": "AlertSettingsIcon",
  "Deposit a Check": "RDCIcon"
}

Then it's just...

$("li.menu-item a.ui-link").each(function() {
    $(this).addClass(map[$(this).text()] || "AlertSettingIcon");
});

Or even better...

$("li.menu-item a.ui-link").addClass(function() {
    return map[$(this).text()] || "AlertSettingIcon";
});

I updated my code with your edits and all the classes are added except for the AlertSettingIcon. I had updated my post referencing your code to try and get help withe Alerts & Settings portion, but reverted it. — David, Mar 18 '15 at 15:49

Ampersand in innerHtml() not being read correctly and validating in JS

1 Answers1