Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2891153002:
Add screen reader markup to the Incognito icon (Closed)
DescriptionAdd screen reader markup to the Incognito icon
Add role="img" to indicate that the div contains an icon, and alt="" to
make screen readers skip it. Adding alt texts such as "Incognito" or
"Incognito icon" is not valuable, as there is a header saying
"You've gone incognito" just under the icon.
BUG=699400
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2891153002
Cr-Commit-Position: refs/heads/master@{#473541}
Committed: https://chromium.googlesource.com/chromium/src/+/20e6606b1bb80a0c6706e61d3756ecd2f1365da3
Patch Set 1 #Patch Set 2 : role=presentation #Messages
Total messages: 20 (13 generated)
Description was changed from ========== Add screen reader markup to the Incognito icon Add role="img" to indicate that the div contains an icon, and alt="" to make screen readers skip it. Adding alt texts such as "Incognito" or "Incognito icon" is not valuable, as there is a header saying "You've gone incognito" just under the icon. BUG=699400 ========== to ========== Add screen reader markup to the Incognito icon Add role="img" to indicate that the div contains an icon, and alt="" to make screen readers skip it. Adding alt texts such as "Incognito" or "Incognito icon" is not valuable, as there is a header saying "You've gone incognito" just under the icon. BUG=699400 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by msramek@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
msramek@chromium.org changed reviewers: + dbeam@chromium.org
Hello again, Dan! Another tiny change to the Incognito NTP, PTAL :) Thanks, Martin
a11y folks disagree with you about the "not valuable" part: https://bugs.chromium.org/p/chromium/issues/detail?id=693386 i don't really, but if you do end up hiding this from screen readers, maybe use role="presentation"?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by msramek@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/05/18 18:41:56, Dan Beam wrote: > a11y folks disagree with you about the "not valuable" part: > https://bugs.chromium.org/p/chromium/issues/detail?id=693386 Actually, if you look at the linked crbug.com/699400, it's the same bug reporter and the same problem statement as your crbug.com/693386, but this time, I got an explicit approval for an empty alt text :) > > i don't really, but if you do end up hiding this from screen readers, maybe use > role="presentation"? Yep, that makes more sense. Thanks!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
fwiw, i don't know if the alt="" matters if you use role="presentation" (I think it basically just tells screen readers to ignore this element) lgtm
Thanks. That makes sense, but due to the somewhat heuristic nature of screen readers, I'd prefer to keep both. For example, this article: https://www.w3.org/WAI/tutorials/images/decorative/ suggests that role="presentation" is not as well supported by screen readers as alt="", and I found a few other articles about accessibility which also use both.
The CQ bit was checked by msramek@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1495443293559570,
"parent_rev": "bac78adb5ee5024d296d1449aca6d3d66c17c89a", "commit_rev":
"20e6606b1bb80a0c6706e61d3756ecd2f1365da3"}
Message was sent while issue was closed.
Description was changed from ========== Add screen reader markup to the Incognito icon Add role="img" to indicate that the div contains an icon, and alt="" to make screen readers skip it. Adding alt texts such as "Incognito" or "Incognito icon" is not valuable, as there is a header saying "You've gone incognito" just under the icon. BUG=699400 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Add screen reader markup to the Incognito icon Add role="img" to indicate that the div contains an icon, and alt="" to make screen readers skip it. Adding alt texts such as "Incognito" or "Incognito icon" is not valuable, as there is a header saying "You've gone incognito" just under the icon. BUG=699400 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2891153002 Cr-Commit-Position: refs/heads/master@{#473541} Committed: https://chromium.googlesource.com/chromium/src/+/20e6606b1bb80a0c6706e61d3756... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/20e6606b1bb80a0c6706e61d3756... |
