HTML5 parsing and error checking #362

jeremy · 2016-11-01T08:08:40Z

Switches from Nokogiri to Nokogumbo. Depends on Nokogumbo >= 1.4.10 for error reporting.

Introduces --report-missing-doctype, defaulting to false, to report on missing <!DOCTYPE html> at the beginning of the document.

Fixes #318

Pending:

spec updates for HTML5 parsing differences
whether to switch to HTML5 checking or introduce it as a new check
fix error line reporting (reported upstream at Nokogiri::HTML5 does not support line numbers rubys/nokogumbo#53)
pending Nokogumbo 2.0.0 release (including Include line numbers from the parser. rubys/nokogumbo#55)

gjtorikian · 2016-11-01T18:33:13Z

Wow, I like a lot of this. I especially like the removal of the Nokogiri hack method--thank you for that!

I'm a bit slammed at work at the moment so I won't be able to get this for a few days. For this task item:

whether to switch to HTML5 checking or introduce it as a new check

I think we should just switch to HTML5 checking. I'd rather documents be modern and correct then continue to support separate HTML4/5 checks.

matkoniecz · 2017-01-25T15:22:01Z

I'm a bit slammed at work at the moment so I won't be able to get this for a few days.

ping?

Gemfile

html-proofer.gemspec

lib/html-proofer/check/html.rb

spec/html-proofer/html_spec.rb

gjtorikian · 2017-02-03T23:39:08Z

I like a lot of the clean-up here, thank you very much!

I am a bit concerned at some of the test changes, primarily where items were failing tests and have now flipped to not fail. Could you explain why that is? Is it because the HTML5 parsing is laxer than the previous HTML4 parsing?

jeremy · 2017-02-04T00:09:54Z

Could you explain why that is? Is it because the HTML5 parsing is laxer than the previous HTML4 parsing?

Precisely! Reflects where HTML5 rules differ from XHTML1/HTML4.

jeremy · 2017-02-04T03:23:31Z

spec/html-proofer/links_spec.rb

    missingLinkHrefFilepath = "#{FIXTURES_DIR}/links/missingLinkHref.html"
    proofer = run_proofer(missingLinkHrefFilepath, :file)
-    expect(proofer.failed_tests.first).to match(/anchor has no href attribute/)
+    expect(proofer.failed_tests).to eq []
  end


HTML5 allows links with no href attribute.

jeremy · 2017-02-04T03:23:57Z

spec/html-proofer/links_spec.rb

    head_link = "#{FIXTURES_DIR}/links/head_link_href_absent.html"
    proofer = run_proofer(head_link, :file)
-    expect(proofer.failed_tests.first).to match(/anchor has no href attribute/)
+    expect(proofer.failed_tests).to eq []
  end



Ditto. HTML5 allows links with no href attribute.

gjtorikian · 2017-02-15T22:54:23Z

@jeremy Would you mind merging master to fix the merge conflict? After that I think this should be good to go. Thank you!

jeremy · 2017-02-15T23:00:27Z

@gjtorikian Rebased.

gjtorikian · 2017-03-04T00:06:13Z

I completely forgot about this! Sorry!

it's from me

gjtorikian · 2017-03-04T00:30:02Z

Unfortunately I still can't accept this just yet. Nokogiri::HTML5 does not appear to preserve the line numbers correctly. I opened rubys/nokogumbo#53 and am blocked on that.

fulldecent · 2018-09-04T02:22:25Z

@jeremy The issue is fixed upstream and now Nokogumbo should be providing the line numbers that you need for this fork to be successful.

Would you be able to continue working on this PR? Is there any assistance you require?

jeremy · 2018-09-04T16:56:28Z

Nokogumbo doesn't include a gemspec, so we can't bundle it from git, and the fix isn't included in the recent 2.0.0.pre.alpha release. That means we can't depend on a fixed version to test this PR, so I branched nokogumbo (rubys/nokogumbo#104) to provide a non-Rakefile-embedded gemspec for temporary use.

Gemfile

html-proofer.gemspec

jeremy · 2018-09-04T17:42:50Z

Note that a bunch of line numbers are still incorrect (line 0), so the upstream fix is likely insufficient.

Another route is to support what we can and defer to the underlying library on specifics, allowing users to parse HTML5 today despite the incomplete support.

Introduces `--report-missing-doctype`, defaulting to false, to report on missing `<!DOCTYPE html>` at the beginning of the document. Depends on Nokogumbo >= 1.4.10 for rubys/nokogumbo#46 Fixes gjtorikian#318.

jeremy · 2018-09-04T23:51:35Z

Switched nokogumbo dep to track git master pending 2.0.0 release.

stevecheckoway · 2019-08-10T04:26:49Z

I filed a PR with Nokogiri to add a line setting API (referenced above) and I added rubys/nokogumbo#122 to track the issue in Nokogumbo.

fulldecent · 2019-09-05T16:40:50Z

Thank you for providing the update. May rubys/nokogumbo#122 please be referenced as a blocker at top?

fulldecent · 2019-09-05T16:59:50Z

We have a path forward with rubys/nokogumbo#130

fulldecent · 2019-10-07T13:51:33Z

In Nokogumbo we are waiting for rubys/nokogumbo#130 so that we can use it here.

fulldecent · 2019-10-07T14:06:28Z

Fixed merge conflicts. And still have a few failing tests.

fulldecent · 2019-11-07T22:02:00Z

Requesting help here to fix unit tests and review this PR

gjtorikian · 2019-11-07T23:37:40Z

For some reason I cannot push to this PR? But here is the diff necessary to fix the tests:

diff --git a/lib/html-proofer/middleware.rb b/lib/html-proofer/middleware.rb
index b1e7eab..a154261 100644
--- a/lib/html-proofer/middleware.rb
+++ b/lib/html-proofer/middleware.rb
@@ -67,7 +67,7 @@ module HTMLProofer
           'response',
           Middleware.options
         ).check_parsed(
-          Nokogiri::HTML(Utils.clean_content(html)), 'response'
+          Nokogiri::HTML5(html), 'response'
         )
 
         if parsed[:failures].length > 0

fulldecent · 2019-11-08T21:55:54Z

@gjtorikian I was able to put that in here jeremy@35f85cb

gjtorikian · 2019-11-09T12:35:49Z

@fulldecent Do you have push access to his repo? Normally I can make commits directly to PRs but I can't seem to manage it with this one. Looks like there's just one failure left.

fulldecent · 2019-11-09T15:52:27Z

@gjtorikian Yes, but I can only do it with the web interface. Here is the link I'm using:

https://github.com/jeremy/html-proofer/tree/html5

fulldecent · 2019-12-09T01:22:38Z

@gjtorikian does this work for you now?

gjtorikian · 2019-12-09T13:05:25Z

I do believe so, but I haven't had a chance to look into it a fix as yet.

gjtorikian · 2019-12-09T15:42:13Z

I believe that should do it. I'll take a look at the merge conflicts as soon as I am able.

gjtorikian · 2019-12-10T11:56:45Z

Thanks everyone. This is looking ready to go. I will merge, release, and write release notes as soon as possible--I only just had a quick ten minutes right now to do all the merge conflict fixes.

codecov-io · 2019-12-10T12:00:26Z

Codecov Report

Merging #362 into master will decrease coverage by 0.2%.
The diff coverage is 91.83%.

@@            Coverage Diff            @@
##           master    #362      +/-   ##
=========================================
- Coverage   98.41%   98.2%   -0.21%     
=========================================
  Files          30      30              
  Lines        1951    1949       -2     
=========================================
- Hits         1920    1914       -6     
- Misses         31      35       +4

Impacted Files	Coverage Δ
lib/html-proofer/configuration.rb	`88.88% <ø> (ø)`	⬆️
lib/html-proofer/middleware.rb	`96.55% <ø> (ø)`	⬆️
lib/html-proofer.rb	`100% <ø> (ø)`	⬆️
spec/html-proofer/command_spec.rb	`100% <100%> (ø)`	⬆️
lib/html-proofer/url_validator.rb	`94.07% <100%> (ø)`	⬆️
spec/html-proofer/links_spec.rb	`99.3% <100%> (ø)`	⬆️
lib/html-proofer/check/links.rb	`92.59% <100%> (-0.1%)`	⬇️
spec/html-proofer/element_spec.rb	`100% <100%> (ø)`	⬆️
lib/html-proofer/utils.rb	`100% <100%> (ø)`	⬆️
spec/html-proofer/scripts_spec.rb	`100% <100%> (ø)`	⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c120b7c...a71aac0. Read the comment docs.

jeremy · 2019-12-10T22:33:58Z

Thanks for taking this to the finish line @gjtorikian!! 🎉

jeremy force-pushed the html5 branch 2 times, most recently from 0473f47 to 0a55dd4 Compare November 2, 2016 05:16

peternewman mentioned this pull request Nov 21, 2016

Travis TODO pidcodes/pidcodes.github.com#184

Open

8 tasks

gjtorikian previously requested changes Feb 3, 2017

View reviewed changes

Gemfile Show resolved Hide resolved

html-proofer.gemspec Outdated Show resolved Hide resolved

lib/html-proofer/check/html.rb Show resolved Hide resolved

spec/html-proofer/html_spec.rb Outdated Show resolved Hide resolved

jeremy commented Feb 4, 2017

View reviewed changes

jeremy force-pushed the html5 branch from 0a55dd4 to 09a5dde Compare February 15, 2017 22:59

jeremy force-pushed the html5 branch from 09a5dde to eb94fef Compare May 17, 2017 16:57

gjtorikian mentioned this pull request May 20, 2017

Replace Nokogiri with something that validates HTML5 #318

Closed

fulldecent mentioned this pull request Nov 14, 2017

Add API to set line number, fixes #1657 sparklemotion/nokogiri#1658

Closed

rafbm mentioned this pull request Jan 20, 2018

Add :max_parse_errors argument to .parse, .get and .fragment with 0 as default value rubys/nokogumbo#65

Merged

jeremy force-pushed the html5 branch from eb94fef to f3f0b01 Compare September 4, 2018 17:26

jeremy requested a review from Floppy as a code owner September 4, 2018 17:26

jeremy commented Sep 4, 2018

View reviewed changes

Gemfile Outdated Show resolved Hide resolved

html-proofer.gemspec Outdated Show resolved Hide resolved

jeremy mentioned this pull request Sep 4, 2018

Toplevel .gemspec rubys/nokogumbo#104

Merged

HTML5 parsing using Nokogumbo

78b3e81

Introduces `--report-missing-doctype`, defaulting to false, to report on missing `<!DOCTYPE html>` at the beginning of the document. Depends on Nokogumbo >= 1.4.10 for rubys/nokogumbo#46 Fixes gjtorikian#318.

jeremy force-pushed the html5 branch from f3f0b01 to 78b3e81 Compare September 4, 2018 23:50

stevecheckoway mentioned this pull request Aug 10, 2019

Add Nokogiri::XML::Node#line= sparklemotion/nokogiri#1918

Merged

Merge branch 'master' into html5

0d660ba

Update middleware.rb

35f85cb

Report errors

72a4421

gjtorikian added 4 commits December 10, 2019 00:41

CI trigger?

4f94fed

Merge branch 'master' into html5

ac93f46

rubocop

891df75

Add doctype to valid doc fixture

14dd2f2

gjtorikian added 3 commits December 10, 2019 12:01

Slight gem fixes

a71aac0

Add missing docs

3ef5af5

rubocop!!

95edfa8

gjtorikian merged commit 1471239 into gjtorikian:master Dec 13, 2019

gjtorikian mentioned this pull request Dec 13, 2019

Is it possible to weaken the nokogumbo dependency to version 1.4 and above? #546

Closed

jeremy deleted the html5 branch March 25, 2020 20:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HTML5 parsing and error checking #362

HTML5 parsing and error checking #362

jeremy commented Nov 1, 2016 •

edited by fulldecent

gjtorikian commented Nov 1, 2016

matkoniecz commented Jan 25, 2017

gjtorikian commented Feb 3, 2017

jeremy commented Feb 4, 2017

jeremy Feb 4, 2017

jeremy Feb 4, 2017

gjtorikian commented Feb 15, 2017

jeremy commented Feb 15, 2017

gjtorikian commented Mar 4, 2017

gjtorikian commented Mar 4, 2017

fulldecent commented Sep 4, 2018 •

edited

jeremy commented Sep 4, 2018

jeremy commented Sep 4, 2018

jeremy commented Sep 4, 2018

stevecheckoway commented Aug 10, 2019

fulldecent commented Sep 5, 2019

fulldecent commented Sep 5, 2019

fulldecent commented Oct 7, 2019

fulldecent commented Oct 7, 2019

fulldecent commented Nov 7, 2019

gjtorikian commented Nov 7, 2019

fulldecent commented Nov 8, 2019

gjtorikian commented Nov 9, 2019

fulldecent commented Nov 9, 2019

fulldecent commented Dec 9, 2019

gjtorikian commented Dec 9, 2019

gjtorikian commented Dec 9, 2019

gjtorikian commented Dec 10, 2019

codecov-io commented Dec 10, 2019 •

edited

jeremy commented Dec 10, 2019

HTML5 parsing and error checking #362

HTML5 parsing and error checking #362

Conversation

jeremy commented Nov 1, 2016 • edited by fulldecent

gjtorikian commented Nov 1, 2016

matkoniecz commented Jan 25, 2017

gjtorikian commented Feb 3, 2017

jeremy commented Feb 4, 2017

jeremy Feb 4, 2017

Choose a reason for hiding this comment

jeremy Feb 4, 2017

Choose a reason for hiding this comment

gjtorikian commented Feb 15, 2017

jeremy commented Feb 15, 2017

gjtorikian commented Mar 4, 2017

gjtorikian commented Mar 4, 2017

fulldecent commented Sep 4, 2018 • edited

jeremy commented Sep 4, 2018

jeremy commented Sep 4, 2018

jeremy commented Sep 4, 2018

stevecheckoway commented Aug 10, 2019

fulldecent commented Sep 5, 2019

fulldecent commented Sep 5, 2019

fulldecent commented Oct 7, 2019

fulldecent commented Oct 7, 2019

fulldecent commented Nov 7, 2019

gjtorikian commented Nov 7, 2019

fulldecent commented Nov 8, 2019

gjtorikian commented Nov 9, 2019

fulldecent commented Nov 9, 2019

fulldecent commented Dec 9, 2019

gjtorikian commented Dec 9, 2019

gjtorikian commented Dec 9, 2019

gjtorikian commented Dec 10, 2019

codecov-io commented Dec 10, 2019 • edited

Codecov Report

jeremy commented Dec 10, 2019

jeremy commented Nov 1, 2016 •

edited by fulldecent

fulldecent commented Sep 4, 2018 •

edited

codecov-io commented Dec 10, 2019 •

edited