Scripts checking

[doktorbro]

WIP

Javascripts are another important source we could check. They behave technically mostly like images, except they can have content. Should I implement more?

[doktorbro]

Should I put scripts fixtures in fixtures/scripts?

[gjtorikian]

Right here is fine. In another PR I will move the files into directories for a cleanup. I'd rather not that happen in this PR as it will make the diff unfairly big. Thanks for asking! ❤️

[doktorbro]

Looks complete to me.

[doktorbro]

Would @content.strip.empty? be better?

[gjtorikian]

Good question, I think so, for completeness sake.

[doktorbro]

Done.

[gjtorikian]

I think this makes sense. Two questions:

1. Is it possible to catch self-closing script tags? Like, <script src="./blah.js" />. This is more a hypothetical question. Such a tag is considered not valid, but many times in my life I've made the mistake (rather than closing with </script>).
2. Could I convince you to do another PR (or maybe within this PR?) for style tags?

[doktorbro]

1. I’ll try it. I hope Nokogiri can distinguish self-closing from empty content.

2. What do you want to check in the style tag? How could it be invalid?

   <style>
   body { color: black; background: white; }
   em { font-style: normal; color: red; }
   </style>
   

[gjtorikian]

What do you want to check in the style tag?

Ugh, sorry, I promise I know how to computer. I meant

<link rel="stylesheet" type="text/css" href="styles.css">

Which is already covered.

[doktorbro]

Nokogiri can not distinguish self-closing node from empty one. This element <script src="./blah.js" /> is already parsed to <script src="./blah.js"></script>. You don’t have the original state of element on this stage. That’s why I would not check for self-closing scripts.

[parkr]

You could use regexp for each script node to see if it's self-closing.

[doktorbro]

@parkr I can’t figure out how because the node is already parsed:

node.to_s == '<script src="./blah.js"></script>'

You can add you solution on top of my PR, if you know how. Keep in mind this Stack Overflow answer.

@gjtorikian, you said in #30 (comment)

I don't think valid HTML is something I'm interested in here.

The self-closing check is HTML validation, so we could spare this check.

[gjtorikian]

I'd rather this be rewritten as:

if script.missing_src?
  self.add_issue "script is empty and has no src attribute"
else
  next if script.ignore?
  if script.remote?
    add_to_external_urls script.src
  else
    self.add_issue("internal script #{script.src} does not exist") unless script.exists?

[doktorbro]

Rebased.