I'm working on a page that shows groups of users. The following JavaScript function is used to show more details about a user (such as email and login) when you click on their name.But there's a twist: clicking on one of those pieces of information will highlight it to make it easier to copy. This is accomplished by adding an in-line onclick function to each span element containing the user info.function showUserDetailsInRow(row, user) {  row.innerHTML = ;  var expanderCell = document.createElement(div);  expanderCell.className = GroupsCell;  expanderCell.innerHTML = -;  var spanOpen = <span style='font-style:normal; cursor:text;' onclick='var r=document.body.createTextRange(); r.moveToElementText(this); r.select();'>,   spanClose = </span>;  var mainCell = document.createElement(div);  mainCell.className = GroupsCell;  mainCell.innerHTML = <b>Name:</b>  + spanOpen + user.displayName + spanClose + <br/> + (user.isDomainGroup ? <b>(Domain Group)</b> : (<b>Email:</b>  + spanOpen + user.email + spanClose + <br/> + <b>Login:</b>  + spanOpen + user.loginName + spanClose + <br/>));  var removeCell = document.createElement(div);  removeCell.className = btn;  removeCell.innerHTML = remove from group;  row.appendChild(expanderCell);  row.appendChild(mainCell);  row.appendChild(removeCell);}This works just as I'd expect, but it itches a bit.I feel dirty putting code in a string. Is there a better way to accomplish this without greatly increasing the complexity of this function? Would it be worthwhile or more efficient to offload that dynamically repeated inline function into a single function definition elsewhere in my script instead?I also feel guilty building the innerHTML of the mainCell element via string concatenation (and using tags like <b> and <br/>); should I instead be creating <div> elements via document.createElement, decorating them with class names, and appending those to mainCell? Is there a benefit (whether it be performance, complexity, flexibility, or the like) to doing it one way or another?Here's a working example of the showUserDetailsInRow() function, along with the displayGroupmembers() function that I use to attach it to the mouseup event on row elements. var exampleGroup = {  Members: [{    displayName: Marcellus Gilmore Edson,    email: mge@peanutbutter.com,    loginName: pbc\\medison,    isDomainGroup: false  }, {    displayName: George Washington Carver,    email: gwc@peanutbutter.com,    loginName: pbc\\gcarver,    isDomainGroup: false  }, {    displayName: John Harvey Kellogg,    email: jhk@peanutbutter.com,    loginName: pbc\\jkellogg,    isDomainGroup: false  }, {    displayName: Mister Peanut,    email: mrpnut@peanutbutter.com,    loginName: pbc\\mpeanut,    isDomainGroup: false  }, {    displayName: pbc\\Battle Creek Sanitarium Members,    email: ,    loginName: pbc\\Battle Creek Sanitarium Members,    isDomainGroup: true  }]}displayGroupMembers(exampleGroup);function showUserDetailsInRow(row, user) {  row.innerHTML = ;  var expanderCell = document.createElement(div);  expanderCell.className = GroupsCell;  expanderCell.innerHTML = -;  var spanOpen = <span style='font-style:normal; cursor:text;' onclick='var r=document.body.createTextRange(); r.moveToElementText(this); r.select(); delete r;'>,    spanClose = </span>;  var mainCell = document.createElement(div);  mainCell.className = GroupsCell;  mainCell.innerHTML = <b>Name:</b>  + spanOpen + user.displayName + spanClose + <br/> + (user.isDomainGroup ? <b>(Domain Group)</b> : (<b>Email:</b>  + spanOpen + user.email + spanClose + <br/> + <b>Login:</b>  + spanOpen + user.loginName + spanClose + <br/>));  var removeCell = document.createElement(div);  removeCell.className = btn;  removeCell.innerHTML = remove from group;  row.appendChild(expanderCell);  row.appendChild(mainCell);  row.appendChild(removeCell);}function displayGroupMembers(group) {  var members = document.getElementById(MembersInnerContainer);  members.innerHTML = ;  for (var i = 0, len = group.Members.length; i < len; i++) {    var memberRow = document.createElement(div);    memberRow.className = i % 2 === 0 ? GroupsRow : GroupsRow Alt;    var memberCell = document.createElement(div);    memberCell.className = GroupsCell;    memberCell.innerHTML = +  + group.Members[i].displayName;    memberRow.appendChild(memberCell);    (function(i, group) {      memberRow.onmouseup = function(e) {        e = window.event || e;        e.target = e.target || e.srcElement;        if (e.target.tagName !== SPAN) {          var isOpen = this.getAttribute(expanded);          if (isOpen && isOpen === true) {            this.setAttribute(expanded, false);            this.innerHTML = ;            var memberCell = document.createElement(div);            memberCell.className = GroupsCell;            memberCell.innerHTML = +  + group.Members[i].displayName;            this.appendChild(memberCell);          } else {            this.setAttribute(expanded, true);            showUserDetailsInRow(this, group.Members[i]);          }        }      }    })(i, group);    members.appendChild(memberRow);  }}.SelectedGroupMembers {  display: inline-block;  width: 90%;  border-width: 1px;  border-style: solid;  margin-top: 10px;  margin-left: auto;  margin-right: auto;  margin-bottom: 10px;  text-align: left;}.GroupsHeader {  width: 100%;  font-weight: bold;  padding-top: 5px;  padding-bottom: 5px;  cursor: default;}.InnerContainer {  width: 100%;  overflow: auto;}.CreateGroupLink {  font-weight: bold;  font-size: 12pt;  padding-bottom: 3px;  cursor: pointer;  border-top-width: 1px;  border-style: solid;  border-left: 0px;  border-right: 0px;  border-bottom: 0px;}.GroupsRow {  width: 100%;  color: black;  cursor: pointer;  text-align: left;}.GroupsCell {  margin-left: 5px;  display: inline-block;  vertical-align: text-top;}.GroupsCell span:hover {  background-color: white;  color: black;}#MembersInnerContainer {  max-height: 500px;}#RefreshGroupMembers {  display: inline-block;  float: right;  padding-left: 5px;  padding-right: 5px;  font-size: 8pt;  margin-right: 5px;}.btn {  padding-left: 4px;  padding-right: 4px;  padding-top: 1px;  padding-bottom: 4px;  cursor: pointer;  color: white;  display: inline-block;  border-width: 1px;  border-style: solid;  margin-bottom: 2px;}.btn:hover {  color: black;}[expanded=true] .GroupsCell {  font-style: italic;  margin-left: 6px;}.GroupsRow .btn {  margin-top: 2em;  vertical-align: text-top;  font-size: 8pt;  margin-left: 5px;}.GreenTheme .btn {  background-color: rgb(90, 110, 90);  border-color: rgb(50, 80, 50);}.GreenTheme .btn:hover {  background-color: rgb(150, 220, 150);}.GreenTheme .GroupsHeader {  color: white;  background-color: rgb(90, 110, 90);}.GreenTheme .InnerContainer {  background-color: rgb(220, 245, 220);}.GreenTheme .SelectedGroupPanel {  border-color: rgb(90, 110, 90);  background-color: rgb(240, 250, 240);  color: black;}.GreenTheme .SelectedgroupMembers {  border-color: rgb(90, 110, 90);}.GreenTheme .CreateGroupLink {  color: rgb(90, 110, 90);  background-color: rgb(250, 255, 250);  border-color: rgb(90, 110, 90);}.GreenTheme .CreateGroupLink:hover {  color: black;  background-color: white;}.GreenTheme .GroupsRow.Alt {  background-color: rgb(250, 255, 250);}.GreenTheme .GroupsRow:hover {  background-color: rgb(165, 225, 165);}<div class=GreenTheme>  <div class=SelectedGroupMembers>    <div class=GroupsHeader>&nbsp;Group Members      <div id=RefreshGroupMembers class=btn>reload</div>    </div>    <div class=InnerContainer id=MembersInnerContainer>      loading group members...    </div>    <div class=CreateGroupLink id=AddMember>      &nbsp;+add user to this group    </div>  </div></div>(Since this code needs to work on IE8, it's clobbering the mouseup event instead of adding an event listener to it for the sake of brevity and performance. Add that to the list of things I feel guilty about.)
Absolve me of my guilt: Dynamically creating spans that highlight on click
javascript;html;css
In the order I spot things (so in no particular order):Don't use innerHTML, especially not with user input. Imagine a user called Bob<script>alert('I am evil');</script>You can use document.createElement() to create elements, document.createTextNode() to create a text node, or element.textContent = ... to set plain text.Don't use explicit styling in your JavaScript. Give it a className and use CSS to style.If you don't use string concatenation to create your elements, you can actually use a proper JavaScript function as the event handler.Even better, you can probably make use of event delegation, since all the elements are of similar functionality.You might also benefit the use of a templating engine such as Handlebars or Mustache.