As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact
Last Comment Bug 761910 - add text attributes testing for HTML5 mark color
: add text attributes testing for HTML5 mark color
[good first bug][mentor=eitan@monoton...
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla16
Assigned To: Andrew Quartey [:drexler]
: alexander :surkov
Depends on:
Blocks: a11ytestdev
  Show dependency treegraph
Reported: 2012-06-05 21:49 PDT by alexander :surkov
Modified: 2012-07-07 11:59 PDT (History)
2 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (2.67 KB, patch)
2012-06-15 13:39 PDT, Andrew Quartey [:drexler]
no flags Details | Diff | Splinter Review
updated_patch (1.78 KB, patch)
2012-06-20 10:58 PDT, Andrew Quartey [:drexler]
eitan: review+
Details | Diff | Splinter Review

Description User image alexander :surkov 2012-06-05 21:49:34 PDT
w3c example: 
<p>Look around and you will find, no-one's really
 <mark>colour</mark> blind.</p>

add it to
similar to other examples in this file
Comment 1 User image Andrew Quartey [:drexler] 2012-06-15 13:39:30 PDT
Created attachment 633655 [details] [diff] [review]

Added tests; passed locally.
Comment 2 User image Eitan Isaacson [:eeejay] 2012-06-18 15:13:43 PDT
Comment on attachment 633655 [details] [diff] [review]

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=""
> +     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.
Comment 3 User image Andrew Quartey [:drexler] 2012-06-18 17:01:34 PDT
(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:, 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".
Comment 4 User image Andrew Quartey [:drexler] 2012-06-20 10:58:08 PDT
Created attachment 634976 [details] [diff] [review]

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.
Comment 5 User image Andrew Quartey [:drexler] 2012-07-06 12:36:37 PDT
review ping.
Comment 6 User image Eitan Isaacson [:eeejay] 2012-07-06 12:38:44 PDT
Comment on attachment 634976 [details] [diff] [review]

Sorry for the delay.
Comment 7 User image Andrew Quartey [:drexler] 2012-07-06 12:54:05 PDT
No problem.
Comment 8 User image Ryan VanderMeulen [:RyanVM] 2012-07-07 07:08:53 PDT
Comment 9 User image Ryan VanderMeulen [:RyanVM] 2012-07-07 11:59:36 PDT

Note You need to log in before you can comment on or make changes to this bug.