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

htmlproofer upgraded major version number #1764

Merged
merged 8 commits into from Jul 16, 2022
Merged

Conversation

plaindocs
Copy link
Contributor

And changed some stuff around.
This is a first pass to make it work.

I'm not sure what happened to the internal domain.

CC @mxsasha @CollierCZ

@plaindocs
Copy link
Contributor Author

plaindocs commented Jul 14, 2022

@mxsasha looks like we've got approx 2000 errors with this config, mostly plain http://, do you have a strong preference for any of the following:

  • turn off that error 👿 (this should be an option but might not be)
  • replace all http: with https: and see how many errors remain?
  • fix them manually

@plaindocs
Copy link
Contributor Author

plaindocs commented Jul 14, 2022

Here are some potentially valid errors

For the Links check, the following failures were found:

* At docs/_build/html/conf/australia/2018/index.html:205:

  'a' tag is missing a reference

* At docs/_build/html/conf/australia/2019/index.html:208:

  'a' tag is missing a reference

* At docs/_build/html/conf/australia/2020/index.html:246:

  'a' tag is missing a reference

* At docs/_build/html/conf/australia/2021/index.html:94:

  'a' tag is missing a reference

* At docs/_build/html/conf/australia/2022/index.html:255:

  'a' tag is missing a reference

* At docs/_build/html/conf/cincinnati/2018/index.html:205:

  'a' tag is missing a reference

* At docs/_build/html/conf/portland/2018/index.html:215:

  'a' tag is missing a reference

* At docs/_build/html/conf/portland/2018/index.html:246:

  'a' tag is missing a reference

* At docs/_build/html/conf/portland/2019/index.html:218:

  'a' tag is missing a reference

* At docs/_build/html/conf/portland/2020/index.html:250:

  'a' tag is missing a reference

* At docs/_build/html/conf/portland/2021/index.html:253:

  'a' tag is missing a reference

* At docs/_build/html/conf/portland/2022/index.html:257:

  'a' tag is missing a reference

* At docs/_build/html/conf/prague/2018/index.html:219:

  'a' tag is missing a reference

* At docs/_build/html/conf/prague/2019/index.html:225:

  'a' tag is missing a reference

* At docs/_build/html/conf/prague/2020/index.html:250:

  'a' tag is missing a reference

* At docs/_build/html/conf/prague/2021/index.html:253:

  'a' tag is missing a reference

* At docs/_build/html/conf/prague/2022/index.html:259:

  'a' tag is missing a reference

* At docs/_build/html/conf/vilnius/2019/index.html:207:

  'a' tag is missing a reference

Which remained after brute forcing all links to https.

I'm out of steam for now anyway

@plaindocs
Copy link
Contributor Author

@plaindocs
Copy link
Contributor Author

Finding out if it is possible to leave the http gjtorikian/html-proofer#727

@plaindocs
Copy link
Contributor Author

plaindocs commented Jul 14, 2022

Oh, there is a temporary option of using sudo gem install html-proofer -v 3.19 in the GitHub action file to stick to older version as well.

@plaindocs
Copy link
Contributor Author

plaindocs commented Jul 15, 2022

OK, that was an unexpected journey. There are at least a couple of band-aids in here that we should revisit:

But for now, this turns link checking back on. :-p

Copy link
Collaborator

@CollierCZ CollierCZ left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! Hope the journey wasn't too demanding.

I had one question, but it's probably not a blocker either way.

Comment on lines 117 to 120
<div class="hero-logo" data-proofer-ignore>
<a href="/conf/eu/2016/">
<img src="../../../../_static/img/2016/site-logo.png" />
</a>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would the error go away if the image had an alt attribute? It seems like it's an empty link for people who don't load images, which isn't ideal for accessibility. Though I doubt there's many people going back to look at the logo from 6 years ago.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I was over enthusiastic in my search replace. The ignore there is intended for the bunch of a tags that don't have a reference, like this one: https://github.com/writethedocs/www/pull/1764/files#diff-7a114b4493925c9b9c38c932134ded5be5f69f6f371dec742f42271428f9d96cR45

We don't check for empty or missing alt tags, because we have so many, although I'd definitely be interested in fixing that, at least in current conferences. What is best practice for alt tags for images that don't mean anything and are just part of the theme?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed the unnecessary changes, thanks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

For those, an empty alt attribute:

<img src="image.png" alt="" />

That signals that it's just decoration and is intentionally blank

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, so we have the following two options for htmlproofer:

  p.option 'ignore_empty_alt', '--ignore-empty-alt', ' If `true`, ignores images with empty/missing alt tags (in other words, `<img alt>` and `<img alt="">` are valid; set this to `false` to flag those)'
  p.option 'ignore_missing_alt', '--ignore-missing-alt', 'If `true`, ignores images with missing alt tags'

And we could run htmlproofer multiple times, for example ignoring alts on everything before 2022, and another not ignoring alts but allowing empty on 2022...

The place to start there would be in docs/_templates/2022/*

@plaindocs plaindocs merged commit bc928d8 into main Jul 16, 2022
@plaindocs plaindocs deleted the fix-html-proofer-4 branch July 16, 2022 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants