history: announce search/time ranges.

history: announce search/time ranges.

R=dmazzoni@chromium.org
BUG=431019
TEST=search for something on history page -> should announce the number of results

Committed: https://crrev.com/a602b40551fe46345e0e39299889ed1d959729d9
Cr-Commit-Position: refs/heads/master@{#305116}

[dmazzoni, 2014-11-07 21:35:08]

What do you think of tweaking existing text on the page and making that text a
live region? That's always preferable to specifically announcing something.

Announcing uses a hidden live region under the hood, but it's preferable to
associate the live region with a real element on the page rather than a hidden
one, if possible.

[Dan Beam, 2014-11-07 22:11:28]

On 2014/11/07 21:35:08, dmazzoni wrote:
> What do you think of tweaking existing text on the page and making that text a
> live region? That's always preferable to specifically announcing something.
> 
> Announcing uses a hidden live region under the hood, but it's preferable to
> associate the live region with a real element on the page rather than a hidden
> one, if possible.

What if we change:

  <!-- 0 search results -->
  <h3>Search results for 'asdf'</h3>
  <ol class="search-results">
    <div class="no-results-message">No search results found.</div>
  </ol>

  <!-- 1+ search results -->
  <h3>Search results for 'asdf'</h3>
  <ol class="search-results">
    <li class="entry">...</li>
  </ol>

to

  <!-- 0 search results -->
  <h3>No search results found for 'asdf'</h3>
  <ol class="search-results"></ol>

  <!-- 1+ search results -->
  <h3>Found 1 search result for 'asdf'</h3>
  <ol class="search-results">
    <li class="entry">...</li>
  </ol>

(just get rid of #no-results-message and change the <h3>'s text)?

[dmazzoni, 2014-11-07 22:17:47]

On 2014/11/07 22:11:28, Dan Beam wrote:
> (just get rid of #no-results-message and change the <h3>'s text)?

Sounds great.

[Dan Beam, 2014-11-20 02:35:37]

Patchset #3 (id:40001) has been deleted

[Dan Beam, 2014-11-20 03:03:04]

Patchset #3 (id:60001) has been deleted

[Dan Beam, 2014-11-20 03:03:20]

Patchset #2 (id:20001) has been deleted

[Dan Beam, 2014-11-20 03:27:11]

Patchset #2 (id:80001) has been deleted

[Dan Beam, 2014-11-20 03:32:22]

ptal

https://codereview.chromium.org/707123002/diff/100001/chrome/browser/resource...
File chrome/browser/resources/history/history.js (left):

https://codereview.chromium.org/707123002/diff/100001/chrome/browser/resource...
chrome/browser/resources/history/history.js:1578: var header =
document.createElement('h3');
one of the reasons why VoiceOver doesn't read this <h3> is that it's dynamically
added:
http://jsfiddle.net/s4176eh5/ doesn't work in Safari
http://jsfiddle.net/s4176eh5/1/ works in Safari

however, when I moved this <h3> to the HTML (so it's always in the DOM)
VoiceOver still doesn't read it 99% of the time (sometimes is does, seems like a
race somewhere...).

for now, should I replace aria-{controls,live} with:

  announceAccessibleMessage($('results-header').textContent);

(more or less) as it works in all screenreaders?

[dmazzoni, 2014-11-20 07:01:00]

On 2014/11/20 03:32:22, Dan Beam wrote:
> one of the reasons why VoiceOver doesn't read this <h3> is that it's
dynamically
> added:

ah yes - VoiceOver is bad at dynamically-added live regions, but even so, it's
not entirely clear what's supposed to happen when you set aria-live and also
change the value of a region in the same epoch. Other screen readers may also
not speak anything, because you want to be able to add aria-live to something
that already has content and not speak the initial content, just speak changes
after that.

> http://jsfiddle.net/s4176eh5/ doesn't work in Safari
> http://jsfiddle.net/s4176eh5/1/ works in Safari
> 
> however, when I moved this <h3> to the HTML (so it's always in the DOM)
> VoiceOver still doesn't read it 99% of the time (sometimes is does, seems like
a
> race somewhere...).

No, let's stick with this version - it ought to work, and I'd like to figure out
why it doesn't work 100% of the time.

[dmazzoni, 2014-11-20 07:01:06]

lgtm

[Dan Beam, 2014-11-20 18:32:16]

The CQ bit was checked by dbeam@chromium.org

[commit-bot: I haz the power, 2014-11-20 18:33:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/707123002/120001

[commit-bot: I haz the power, 2014-11-20 19:32:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-11-20 19:32:04]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Dan Beam, 2014-11-20 22:01:02]

The CQ bit was checked by dbeam@chromium.org

[commit-bot: I haz the power, 2014-11-20 22:02:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/707123002/120002

[commit-bot: I haz the power, 2014-11-21 00:10:19]

Committed patchset #4 (id:120002)

[commit-bot: I haz the power, 2014-11-21 00:12:02]

Patchset 4 (id:??) landed as
https://crrev.com/a602b40551fe46345e0e39299889ed1d959729d9
Cr-Commit-Position: refs/heads/master@{#305116}