Check usemap attribute of <img> elements #64
Conversation
Codecov Report
@@ Coverage Diff @@
## master #64 +/- ##
==========================================
+ Coverage 95.67% 95.82% +0.14%
==========================================
Files 17 17
Lines 902 934 +32
==========================================
+ Hits 863 895 +32
Misses 34 34
Partials 5 5
Continue to review full report at Codecov.
|
|
@igneus This looks really good. I'll review this week. |
htmltest/check-img.go
Outdated
| if parent.Data == "a" { | ||
| hT.issueStore.AddIssue(issues.Issue{ | ||
| Level: issues.LevelError, | ||
| Message: "img with usemap in a link", |
There was a problem hiding this comment.
Perhaps tweak the wording to explain this isn't allowed
<img> with usemap attribute not allowed as a descendant of am <a> tag
htmltest/check-img.go
Outdated
| } else if parent.Data == "button" { | ||
| hT.issueStore.AddIssue(issues.Issue{ | ||
| Level: issues.LevelError, | ||
| Message: "img with usemap in a button", |
| func TestImageUsemapMapDoesNotExist(t *testing.T) { | ||
| // detects usemap pointing to a non-existent map | ||
| hT := tTestFile("fixtures/images/usemapMapDoesNotExist.html") | ||
| tExpectIssueCount(t, hT, 1) |
There was a problem hiding this comment.
Alongside tExpectIssueCount please also use tExpectIssue to ensure the correct issue is caught
(tExpectIssue(t, hT, "message here", 1))
|
@wjdp All three issues mentioned in the review reflected. |
| func TestImageUsemapReferenceInvalid(t *testing.T) { | ||
| // detects usemap with reference formally invalid | ||
| hT := tTestFile("fixtures/images/usemapReferenceInvalid.html") | ||
| tExpectIssueCount(t, hT, 1) |
There was a problem hiding this comment.
Could you put in a tExpectIssue here?
| func TestImageUsemapEmpty(t *testing.T) { | ||
| // detects empty usemap | ||
| hT := tTestFile("fixtures/images/usemapEmpty.html") | ||
| tExpectIssueCount(t, hT, 1) |
There was a problem hiding this comment.
Could you put in a tExpectIssue here?
| func TestImageUsemapInLink(t *testing.T) { | ||
| // detects forbidden usemap in an <a> alement | ||
| hT := tTestFile("fixtures/images/usemapInLink.html") | ||
| tExpectIssueCount(t, hT, 1) |
There was a problem hiding this comment.
Could you put in a tExpectIssue here?
| func TestImageUsemapInButton(t *testing.T) { | ||
| // detects forbidden usemap in a <button> alement | ||
| hT := tTestFile("fixtures/images/usemapInButton.html") | ||
| tExpectIssueCount(t, hT, 1) |
There was a problem hiding this comment.
Could you put in a tExpectIssue here?
|
@wjdp Sure. I was surprised myself as I saw that most tests check just issue count and don't match detected issues specifically. But then I followed what seemed to be more or less standard way of writing tests in this project. |
an attempt to implement #19