[Bug 990816] HTML5 Validation For Questions #2407
Conversation
Added empty alt attribute
| <time itemprop="dateCreated" datetime="{{ question.created }}">{{ datetimeformat(question.created) }}</time> | ||
| <span itemprop="dateCreated"> | ||
| {{ datetimeformat(question.created) }} | ||
| </span> |
There was a problem hiding this comment.
You're using tabs for indentation here. They should be spaces instead.
There was a problem hiding this comment.
Ok, fixed and pushed the changes.
| <time itemprop="dateCreated" datetime="{{ question.created }}">{{ datetimeformat(question.created) }}</time> | ||
| <span itemprop="dateCreated"> | ||
| {{ datetimeformat(question.created) }} | ||
| </span> |
There was a problem hiding this comment.
I don't think this change will do what we want, though I'm not sure. The itemprop attribute is used as part of a microdata schema. I think you may need to modify datetimeformat to add extra parameters like itemprop.
@rehandalal You implemented the microdata stuff, right? Will it be fine to have the itemprop attribute on a surrounding <span> instead of on the <time> element that datetimeformat generates?
There was a problem hiding this comment.
no we specifically want this in a <time> element. it is also a valid html5 tag.
There was a problem hiding this comment.
A time element is still being created via datetimeformat(question.created), I only removed the outer time element. The parser looks for what is nested inside of the element that contains itemprop as well. It validates correctly using https://developers.google.com/structured-data/testing-tool/ , I obviously defer judgement to you guys, but that is the reasoning behind my changes.
There was a problem hiding this comment.
yeah sorry my bad, i see that now. basically we need that itemprop attribute to be on the <time>. so perhaps datetimeformat will need to be updated. i need some time to dig around and figure out what should happen here.
There was a problem hiding this comment.
@rehandalal Any update here?
|
Sorry about the delay responding to this... I think we need to update def datetimeformat(context, value, format='shortdatetime', attrs=None):
# attrs is a dict of attributes to be added to the <time> element.Then we would use something like Does that make sense? |
|
This PR is pretty old and there's no way it can get merged in now. I think if we want to let it go forward, it should get rebased against master and the issues need to be resolved. Otherwise we should close it out and a new PR with updated changes should get created. |
|
Please file a new PR with an updated version of this change, or parts of it. If we leave out the bit about the microdata, the rest of this could probably land easily. |
Since every single template file would need validated, I feel that it is beyond the scope of a single bug. That being said, I fixed all the errors I found except for one, which was out of my control since it is dynamically created in:
line 199 questions/templates/questions/includes/aaq_macros.htmlThe
{{ url }}outputs two anchor links separated by a comma which the validator doesn't appreciate very much. I created the pull request since I corrected the errors listed initially with the bug report.