The default bug view has changed. See this FAQ.

add text attributes testing for HTML5 mark color

RESOLVED FIXED in mozilla16

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: surkov, Assigned: drexler)

Tracking

(Blocks: 1 bug)

unspecified
mozilla16
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=eitan@monotonous.org][lang=js])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
w3c example: 
<p>Look around and you will find, no-one's really
 <mark>colour</mark> blind.</p>

add it to http://mxr.mozilla.org/mozilla-central/source/accessible/tests/mochitest/attributes/test_text.html?force=1
similar to other examples in this file
(Reporter)

Updated

5 years ago
Whiteboard: [good first bug][mentor=eitan@monotonous.org][lang=c++] → [good first bug][mentor=eitan@monotonous.org][lang=js]
(Assignee)

Comment 1

5 years ago
Created attachment 633655 [details] [diff] [review]
patch

Added tests; passed locally.
Assignee: nobody → andrew.quartey
Status: NEW → ASSIGNED
Attachment #633655 - Flags: review?(eitan)
Comment on attachment 633655 [details] [diff] [review]
patch

Review of attachment 633655 [details] [diff] [review]:
-----------------------------------------------------------------

Overall looks good. Please address the two issues below (either with a new patch or an explanation that I am wrong).

::: accessible/tests/mochitest/attributes/test_text.html
@@ +534,5 @@
> +      attrs = {};
> +      testTextAttrs(ID, 0, attrs, defAttrs, 0, 10);
> +      
> +      // Walk to the span with the different background color
> +      tempElem = getNode(ID).firstChild.nextSibling;

Can't you just give the <mark> tag an ID and get it that way? Seems more readable and less vulnerable to future breakage.

@@ +585,5 @@
> +  <a target="_blank"
> +     href="https://bugzilla.mozilla.org/show_bug.cgi?id=761910"
> +     title="Add text attributes testing for HTML5 mark color">
> +    Mozilla Bug 761910
> +  </a>

No need to add a reference to this bug since it is not the implementation bug, just the test bug.
Attachment #633655 - Flags: review?(eitan)
(Assignee)

Comment 3

5 years ago
(In reply to Eitan Isaacson [:eeejay] from comment #2)

> Can't you just give the <mark> tag an ID and get it that way? Seems more
> readable and less vulnerable to future breakage.

The reason for it is that according to the spec: http://www.whatwg.org/specs/web-apps/current-work/multipage/text-level-semantics.html#the-mark-element, the mark element is used within a quotation or block of text. Should i give it an ID, i will then have it as a tag within the body of the test. Another alternative will be to append it to a different test block, say "area10".
(Assignee)

Comment 4

5 years ago
Created attachment 634976 [details] [diff] [review]
updated_patch

From the DOM Inspector, i found the mark element to be a text leaf, likewise the span element. Since the test function testTextAttrs will eventually make calls to determine whether the element is accessible or not, failing in the latter case; directly accessing the element by its id doesn't seem to be the way to go. I think that's why other tests in the file involving the span element similarly avoided doing so.
Attachment #633655 - Attachment is obsolete: true
Attachment #634976 - Flags: review?(eitan)
(Assignee)

Comment 5

5 years ago
review ping.
Comment on attachment 634976 [details] [diff] [review]
updated_patch

Sorry for the delay.
Attachment #634976 - Flags: review?(eitan) → review+
(Assignee)

Comment 7

5 years ago
No problem.
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/fa518b4a8222
Flags: in-testsuite-
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/fa518b4a8222
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
You need to log in before you can comment on or make changes to this bug.