Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Link checker: Check for double forward slashes #3562

Merged
merged 8 commits into from
Jan 15, 2019

Conversation

jeff-matthews
Copy link
Contributor

This PR is a:

  • New topic
  • Content update
  • Content fix or rewrite
  • Bug fix or improvement

Summary

When this pull request is merged, it will add a custom test called DoubleSlashCheck to extend the html-proofer library, since the library doesn't natively check for this scenario. See gjtorikian/html-proofer#386.

URLs with double slashes will cause 404s on our website because our hosting solution doesn't automatically strip extra forward slashes from URLs like normal web servers.

This PR also fixes existing instances of double forward slashes in URLs.

@jeff-matthews jeff-matthews self-assigned this Jan 12, 2019
@jeff-matthews jeff-matthews added the Site Improvements Updates to tools, processes, and site architecture that improve reader and contributor experience label Jan 12, 2019
@jeff-matthews jeff-matthews changed the title Fix broken links:DoubleSlashChecker Link checker: Check for double forward slashes Jan 12, 2019
Copy link
Collaborator

@dshevtsov dshevtsov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot reproduce the test.
Debugging shows that class DoubleSlashCheck does not participate in the link checker run.
What is the scenario and expected behavior for failing test?

@jeff-matthews
Copy link
Contributor Author

The scenario is a writer adding an extra forward slash when entering a URL in an <a> element (e.g., after a Jekyll variable). The LinkChecker class will not identify this as a failure, so we end up with 404s on the website despite the LinkCheck class. That's why we need a custom test/class.

The expected behavior of a failing test is any occurrence of a double forward slash in an <a> element in the compiled HTML (excluding double slashes around the protocol like https://).

For example:

Fail
<a href="https://devdocs.magento.com//path/to/file.html">Fail</a>

Pass
<a href="https://devdocs.magento.com/path/to/file.html">Pass</a>

The new class actually runs with the other checks (e.g., ImageCheck, ScriptCheck, LinkCheck, DoubleSlashCheck).

@dshevtsov
Copy link
Collaborator

dshevtsov commented Jan 14, 2019

The new class doesn't run on my environment, I still see only ImageCheck, ScriptCheck, LinkCheck.
My debugging tool shows that there are no entry points to the new class when the link checker is running.

@jeff-matthews
Copy link
Contributor Author

Have you tried running rake test?

@dshevtsov
Copy link
Collaborator

ah, now I see it
so, it is not available for the link checking hook

@jeff-matthews
Copy link
Contributor Author

Oops! That wasn't my intention. I assumed that the hook would invoke the new class. Let me know how we can change or optimize the new class. I was just looking for the quickest way to get this working. I'm open to suggestions.

@dshevtsov
Copy link
Collaborator

Working on it

@jeff-matthews
Copy link
Contributor Author

running tests

@jeff-matthews jeff-matthews merged commit edfd938 into master Jan 15, 2019
@jeff-matthews jeff-matthews deleted the custom-link-check-test branch January 15, 2019 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Site Improvements Updates to tools, processes, and site architecture that improve reader and contributor experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants