Closed
Bug 761910
Opened 11 years ago
Closed 11 years ago
add text attributes testing for HTML5 mark color
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla16
People
(Reporter: surkov, Assigned: drexler)
References
(Blocks 1 open bug)
Details
(Whiteboard: [good first bug][mentor=eitan@monotonous.org][lang=js])
Attachments
(1 file, 1 obsolete file)
1.78 KB,
patch
|
eeejay
:
review+
|
Details | Diff | Splinter Review |
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•11 years ago
|
Whiteboard: [good first bug][mentor=eitan@monotonous.org][lang=c++] → [good first bug][mentor=eitan@monotonous.org][lang=js]
Assignee | ||
Comment 1•11 years ago
|
||
Added tests; passed locally.
Comment 2•11 years ago
|
||
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•11 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•11 years ago
|
||
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•11 years ago
|
||
review ping.
Comment 6•11 years ago
|
||
Comment on attachment 634976 [details] [diff] [review] updated_patch Sorry for the delay.
Attachment #634976 -
Flags: review?(eitan) → review+
Comment 8•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/fa518b4a8222
Flags: in-testsuite-
Keywords: checkin-needed
Comment 9•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/fa518b4a8222
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
You need to log in
before you can comment on or make changes to this bug.
Description
•