1064130 - Improve handling of a11y labels for autocomplete results

Status: RESOLVED FIXED

(Toolkit :: Places, defect)

Description

[Blair McBride [:Unfocused] (UNAVAILABLE)]

autocomplete.xml supports an optional API to be implemented by an inheriting binding to add a label to a result for a11y.

So we get something like:
  "My webpage title http://www.example.com/ Bookmark

Unfortunately, determining what that last token should be is a bit fragile at the moment - and bug 951624 makes it just plain broken by adding new types of actions.

Comment 1

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Marco: FYI, this is split off from bug 951624.

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1

Awaiting Try results:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=88c65c3ce97f

Because:
* We have *no* test coverage for the autocomplete result binding
* Linux hates popups and behaves differently on our build machines than it does for me locally

Comment 3

[Marco Mucci [:MarcoM]]

Added to IT 35.1.  Blair, can you add the point and qe-verify mark.

(In reply to Blair McBride [:Unfocused] from comment #1)
> Marco: FYI, this is split off from bug 951624.

Comment 4

[Marco Bonardo [:mak]]

Comment on attachment 8485686 [details] [diff] [review]
Patch v1

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

::: browser/base/content/test/general/browser_autocomplete_a11y_label.js
@@ +1,2 @@
> +/* Any copyright is dedicated to the Public Domain.
> + * http://creativecommons.org/publicdomain/zero/1.0/ */

I was thinking we could also have a simpler xpcshell test in unifiedcomplete/ checking all of the styles.

The only test we have so far is this one, that is good but incomplete, maybe you could improve it here too.
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js#169

Btw, it's not a request here, this test looks fine and verifies the attribute. Mine was just a thought, maybe we should have a bug to make it real.

@@ +14,5 @@
> +  return deferred.promise;
> +}
> +
> +
> +function* check_a11y_label(input, expectedLabel) {

input may be confused with the input field, what about inputText or searchText

@@ +33,5 @@
> +  gURLBar.value = input.slice(0, -1);
> +  EventUtils.synthesizeKey(input.slice(-1) , {});
> +  yield searchDeferred.promise;
> +  yield promisePopupShown(gURLBar.popup);
> +//debugger;

leftover, I suppose

@@ +36,5 @@
> +  yield promisePopupShown(gURLBar.popup);
> +//debugger;
> +  let firstResult = gURLBar.popup.richlistbox.firstChild;
> +  is(firstResult.getAttribute("type"), "action switchtab", "Expect right type attribute");
> +  is(firstResult.label, expectedLabel, "Result a11y label should be as expected");

well, I guess removeTab is enough to close the popup, but I'd do regardless for coherence

Comment 5

[Marco Mucci [:MarcoM]]

Hi Blair, can you provide a point value and mark it as qe-verify+ or qe-verify-.

Comment 6

[Marco Bonardo [:mak]]

I forgot one thing, the b-c test should ensure unified complete is enabled, otherwise if we should ever disable it it will start to fail.

Comment 7

[Blair McBride [:Unfocused] (UNAVAILABLE)]

*sigh* I keep forgetting to set these lately.

Comment 8

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to Marco Bonardo [:mak] (needinfo? me) from comment #4)
> I was thinking we could also have a simpler xpcshell test in
> unifiedcomplete/ checking all of the styles.

Filed bug 1065191, will pick that up now.




https://hg.mozilla.org/integration/fx-team/rev/f3919c1c302e

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f3919c1c302e