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.

Sign up for GitHub

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

Check usemap attribute of <img> elements #64

Merged
merged 5 commits into from
Oct 12, 2017
Merged

Check usemap attribute of <img> elements #64

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6a567d2
check img usemap attribute
igneus Oct 8, 2017
1a5f81a
img usemap: parent check
igneus Oct 8, 2017
5ac9f7e
improve wording of error messages
igneus Oct 12, 2017
b5168ea
check for a specific error message
igneus Oct 12, 2017
f7a3d5d
check for specific error messages
igneus Oct 12, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 37 additions & 1 deletion htmltest/check-img.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (

func (hT *HTMLTest) checkImg(document *htmldoc.Document, node *html.Node) {
attrs := htmldoc.ExtractAttrs(node.Attr,
[]string{"src", "alt", hT.opts.IgnoreTagAttribute})
[]string{"src", "alt", "usemap", hT.opts.IgnoreTagAttribute})

// Ignore if data-proofer-ignore set
if htmldoc.AttrPresent(node.Attr, hT.opts.IgnoreTagAttribute) {
Expand Down Expand Up @@ -64,6 +64,42 @@ func (hT *HTMLTest) checkImg(document *htmldoc.Document, node *html.Node) {
return
}

// Check usemap
if htmldoc.AttrPresent(node.Attr, "usemap") {
usemapRef := htmldoc.NewReference(document, node, attrs["usemap"])

if len(usemapRef.URL.Path) > 0 {
hT.issueStore.AddIssue(issues.Issue{
Level: issues.LevelError,
Message: "only fragment starting with # allowed in usemap attribute",
Reference: ref,
})
} else if len(usemapRef.URL.Fragment) == 0 {
hT.issueStore.AddIssue(issues.Issue{
Level: issues.LevelError,
Message: "usemap empty",
Reference: ref,
})
} else {
hT.checkInternalHash(usemapRef)
}

parent := node.Parent
if parent.Data == "a" {
hT.issueStore.AddIssue(issues.Issue{
Level: issues.LevelError,
Message: "<img> with usemap attribute not allowed as descendant of an <a> element",
Reference: ref,
})
} else if parent.Data == "button" {
hT.issueStore.AddIssue(issues.Issue{
Level: issues.LevelError,
Message: "<img> with usemap attribute not allowed as descendant of a <button>",
Reference: ref,
})
}
}

// Route reference check
switch ref.Scheme() {
case "http":
Expand Down
41 changes: 41 additions & 0 deletions htmltest/check-img_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,47 @@ func TestImagePre(t *testing.T) {
tExpectIssueCount(t, hT, 0)
}

func TestImageUsemap(t *testing.T) {
// deals with valid usemap
hT := tTestFile("fixtures/images/usemapValid.html")
tExpectIssueCount(t, hT, 0)
}

func TestImageUsemapMapDoesNotExist(t *testing.T) {
// detects usemap pointing to a non-existent map
hT := tTestFile("fixtures/images/usemapMapDoesNotExist.html")
tExpectIssueCount(t, hT, 1)
Copy link
Owner

Choose a reason for hiding this comment

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

Alongside tExpectIssueCount please also use tExpectIssue to ensure the correct issue is caught

(tExpectIssue(t, hT, "message here", 1))

tExpectIssue(t, hT, "hash does not exist", 1)
}

func TestImageUsemapReferenceInvalid(t *testing.T) {
// detects usemap with reference formally invalid
hT := tTestFile("fixtures/images/usemapReferenceInvalid.html")
tExpectIssueCount(t, hT, 1)
Copy link
Owner

Choose a reason for hiding this comment

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

Could you put in a tExpectIssue here?

tExpectIssue(t, hT, "only fragment starting with # allowed", 1)
}

func TestImageUsemapEmpty(t *testing.T) {
// detects empty usemap
hT := tTestFile("fixtures/images/usemapEmpty.html")
tExpectIssueCount(t, hT, 1)
Copy link
Owner

Choose a reason for hiding this comment

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

Could you put in a tExpectIssue here?

tExpectIssue(t, hT, "usemap empty", 1)
}

func TestImageUsemapInLink(t *testing.T) {
// detects forbidden usemap in an <a> alement
hT := tTestFile("fixtures/images/usemapInLink.html")
tExpectIssueCount(t, hT, 1)
Copy link
Owner

Choose a reason for hiding this comment

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

Could you put in a tExpectIssue here?

tExpectIssue(t, hT, "usemap attribute not allowed as descendant of an <a>", 1)
}

func TestImageUsemapInButton(t *testing.T) {
// detects forbidden usemap in a <button> alement
hT := tTestFile("fixtures/images/usemapInButton.html")
tExpectIssueCount(t, hT, 1)
Copy link
Owner

Choose a reason for hiding this comment

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

Could you put in a tExpectIssue here?

tExpectIssue(t, hT, "usemap attribute not allowed as descendant of a <button>", 1)
}

func TestImageMultipleProblems(t *testing.T) {
hT := tTestFile("fixtures/images/multipleProblems.html")
tExpectIssueCount(t, hT, 6)
Expand Down
9 changes: 9 additions & 0 deletions htmltest/fixtures/images/usemapEmpty.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<html>

<body>

<img usemap="" src="./gpl.png" alt="alt text" />

</body>

</html>
14 changes: 14 additions & 0 deletions htmltest/fixtures/images/usemapInButton.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<html>

<body>

<map name="primary">
<area shape="circle" coords="75,75,75" href="usemapMapDoesNotExist.html">
</map>
<button>
<img usemap="#primary" src="gpl.png" alt="GPL">
</button>

</body>

</html>
14 changes: 14 additions & 0 deletions htmltest/fixtures/images/usemapInLink.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<html>

<body>

<map name="primary">
<area shape="circle" coords="75,75,75" href="usemapMapDoesNotExist.html">
</map>
<a href="usemapValid.html">
<img usemap="#primary" src="gpl.png" alt="GPL">
</a>

</body>

</html>
9 changes: 9 additions & 0 deletions htmltest/fixtures/images/usemapMapDoesNotExist.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<html>

<body>

<img usemap="#map" src="./gpl.png" alt="alt text" />

</body>

</html>
12 changes: 12 additions & 0 deletions htmltest/fixtures/images/usemapReferenceInvalid.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<html>

<body>

<map name="primary">
<area shape="circle" coords="75,75,75" href="usemapMapDoesNotExist.html">
</map>
<img usemap="primary" src="gpl.png" alt="GPL">

</body>

</html>
12 changes: 12 additions & 0 deletions htmltest/fixtures/images/usemapValid.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<html>

<body>

<map name="primary">
<area shape="circle" coords="75,75,75" href="usemapMapDoesNotExist.html">
</map>
<img usemap="#primary" src="gpl.png" alt="GPL">

</body>

</html>