6
\$\begingroup\$

I'm using the following code to parse html, This code is working but since I'm new to JavaScript and working in alone project I would appreciate if you can give me some tips and feedback how can I improve this code,or it is fine...

The requirements are:

  1. Insert a new script after given script ID(in the code after test-ui-bootstrap')
  2. For given script ID update attribute value (when the key is given)

code need to update script content after existing script with new content. The code is working and I use part of it from SO past post

https://jsfiddle.net/st6xg7Lj/1/

Here is the code to convert and convert back

function parseHtml(html) {

  // replace html, head and body tag with html_temp, head_temp and body_temp
  html = html.replace(/<!DOCTYPE HTML>/i, '<doctype></doctype>');
  html = html.replace(/(<\/?(?:html)|<\/?(?:head)|<\/?(?:body))/ig, '$1_temp');

  // wrap the dom into a <container>: the html() function returns only the contents of an element
  html = "<container>"+html+"</container>"; 
  var element = $(html); // parse the html

  return element;
}

function convertBackToHtml(element) {

  // reset the initial changes (_temp)
  var extended_html = element.html();
  extended_html = extended_html.replace(/<doctype><\/doctype>/, '<!DOCTYPE HTML>');
  extended_html = extended_html.replace(/(<\/?html)_temp/ig, '$1');
  extended_html = extended_html.replace(/(<\/?head)_temp/ig, '$1');
  extended_html = extended_html.replace(/(<\/?body)_temp/ig, '$1');

  // replace all &quot; inside data-something=""
  while(extended_html.match(/(<.*?\sdata.*?=".*?)(&quot;)(.*?".*?>)/g)) {
    extended_html = extended_html.replace(/(<.*?\sdata.*?=".*?)(&quot;)(.*?".*?>)/g, "$1'$3");
  }

  return extended_html;
}

Here is the code to create new script with content and update the values

var html = $('textarea.input').val();

  // parse the html to an element
  var element = parseHtml(html);

  // do your calculations on the parsed html
  $("<script>alert(\"test\");<\/script>").insertAfter(element.find('#test-ui-bootstrap'));
  element.find("#test-ui-bootstrap").attr('data-test-ui-libs123', "test.bbb");
  element.find("#test-ui-bootstrap").attr('src', 'resources/aaaa/test-ui-core.js');

  // convert the element back to html
    var extended_html = convertBackToHtml(element);

Since I need to run this code in production very soon and Im fairly new In JS I'd like to get your feedback,I really need it.

The code itself is working OK

\$\endgroup\$
7
  • 2
    \$\begingroup\$ This may be a helpful read \$\endgroup\$ – Dan Apr 7 '16 at 12:02
  • \$\begingroup\$ someone ought to convert this to a snippet from the jsfiddle if possible \$\endgroup\$ – Pimgd Apr 7 '16 at 12:10
  • \$\begingroup\$ @DanPantry - Thank you , Well there is a lot opinions...,you provide this link since you think that using regex is not good?if so, there is Better alternatives to attack my problem ? \$\endgroup\$ – Jenny M Apr 7 '16 at 12:37
  • 1
    \$\begingroup\$ @JennyM I actually posted it as a joke :-) \$\endgroup\$ – Dan Apr 7 '16 at 12:48
  • 1
    \$\begingroup\$ @DanPantry - LOL , so do you think that the code is OK? \$\endgroup\$ – Jenny M Apr 7 '16 at 12:59
2
\$\begingroup\$

To solve the root of your problem, I suggest you actually modify the source of that HTML. Add your scripts and attributes there instead of creating this voodoo with JS.

Now if this is the only way you could do it (sigh)...

First, jQuery cannot parse full HTML and I think you're aware of that. It's also mentioned in the documentation how it creates jQuery objects for complex markup.

If the HTML is more complex than a single tag without attributes, as it is in the above example, the actual creation of the elements is handled by the browser's .innerHTML mechanism. In most cases, jQuery creates a new element and sets the innerHTML property of the element to the HTML snippet that was passed in.

An alternative way to do it is to just skip all this work and just inject an external script instead. You could look for </body> or some element in the markup that's guaranteed to always be there, and replace it with markup containing your external script.

var htmlWithInjectedScript = html.replace('</body>', '<script src="path/to/script.js"></script></body>');

This way, you don't have to mangle the markup while having a place to run JavaScript in all its glory. This assumes that the markup is actually executed somewhere, not just for show and tell and that your script is in a location that's always available.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Thank you,its not that simple :-) (sometimes Im adding script reference as you did (I didnt add it to the question since is too simple...) the other requirements is to insert script inline like I did so, currently two questions: assume that the Original html contain 3 scripts and you need to add the new script after the first existing script , how would you do it? 2. assume that you need to update property value on existing script (like in the example)how would you handle it ?Example will be very helpful , Thanks for assistance :) \$\endgroup\$ – Jenny M Apr 7 '16 at 13:21
2
\$\begingroup\$

Just a few ways you can shorten the code if you want to... (it would work in exactly the same way):

You can string replaces one after the other, like this:

html = html.replace(/<!DOCTYPE HTML>/i, '<doctype></doctype>')
           .replace(/(<\/?(?:html)|<\/?(?:head)|<\/?(?:body))/ig, '$1_temp');

and

extended_html = extended_html.replace(/<doctype><\/doctype>/, '<!DOCTYPE HTML>')
                             .replace(/(<\/?html)_temp/ig, '$1');
                             .replace(/(<\/?head)_temp/ig, '$1');
                             .replace(/(<\/?body)_temp/ig, '$1');

jQuery lets you can string almost anything! Most functions (if not all) return the element they change, so this:

element.find("#test-ui-bootstrap").attr('data-test-ui-libs123', "test.bbb");
element.find("#test-ui-bootstrap").attr('src', 'resources/aaaa/test-ui-core.js');

can turn into:

element.find("#test-ui-bootstrap").attr('data-test-ui-libs123', "test.bbb")
                                  .attr('src', 'resources/aaaa/test-ui-core.js');

It also doesn't find() the element twice so should be quicker :)


Also, about your wrap the dom into a <container>: the html() function returns only the contents of an element comment, you can use .outerHTML() to get the html and the parent.

\$\endgroup\$
6
  • \$\begingroup\$ HI , Thank you! i'll take your tips &voted up:) ,assume that you need to write this code according to the requirements How would you write it? it will be great if you can update the fiddle , I just want to get additional perspective from expert since Im fairly new to JS.waiting for you response Thanks again! \$\endgroup\$ – Jenny M Apr 7 '16 at 22:12
  • \$\begingroup\$ @JennyM I am not, in any way, an expert, but I can help you as much as I can! What do you mean by 'according to the requirements'? \$\endgroup\$ – ᔕᖺᘎᕊ Apr 7 '16 at 22:14
  • \$\begingroup\$ Please see my updated post from last min... \$\endgroup\$ – Jenny M Apr 7 '16 at 22:16
  • 1
    \$\begingroup\$ @JennyM here's an updated fiddle: jsfiddle.net/st6xg7Lj/2, but I think you should post on Stack Overflow for help with your requirements... \$\endgroup\$ – ᔕᖺᘎᕊ Apr 7 '16 at 22:18
  • \$\begingroup\$ Thanks a lot, I dont think I should Post in SO since the code is working I think here is the platform to get feedback how to write code better (maybe I miss something here),my question is if you need to do it , this is the way you go? Thanks a lot ! \$\endgroup\$ – Jenny M Apr 7 '16 at 22:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.