1491210 - stop using nsIDOMXULLabeledControlElement

Status: RESOLVED FIXED

(Core :: Disability Access APIs, task)

Comment 1

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 2

[James Teh [:Jamie]]

Comment on attachment 9009013 [details] [diff] [review]
patch

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

::: accessible/base/nsTextEquivUtils.cpp
@@ +310,5 @@
> +    if (aContent->NodeInfo()->Equals(nsGkAtoms::label, kNameSpaceID_XUL)) {
> +      aContent->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::value,
> +                                     textEquivalent);
> +    }
> +    else {

nit: } else { should be on same line.

Comment 3

[alexander :surkov (:asurkov)]

Attachment: patch

here's more accurate version

Comment 4

[alexander :surkov (:asurkov)]

Attachment: patch

this one is even more accurate :) I didn't realized that autocomplete-items implements their own nsIDOMXULSelectControlItemElement::label property (https://dxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/autocomplete.xml#1346).

Comment 5

[alexander :surkov (:asurkov)]

Attachment: patch

sorry for the spam, one more try

Comment 6

[James Teh [:Jamie]]

Comment on attachment 9009842 [details] [diff] [review]
patch

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

::: accessible/generic/Accessible.cpp
@@ +803,5 @@
>     */
>  
>    // CASE #1 (via label attribute) -- great majority of the cases
> +  nsCOMPtr<nsIDOMXULSelectControlElement> select = do_QueryInterface(aElm);
> +  if (!select &&

nit: As we did previously, it'd be good to have a comment here explaining why we exclude select elements; i.e. because the label attribute is used to indicate which option is selected.

Comment 7

[James Teh [:Jamie]]

Oh damn, I didn't see the last patch. Hang on.

Comment 8

[James Teh [:Jamie]]

So nsIDOMXULSelectControlItemElement::GetLabel is not the same as the label attribute?

Comment 9

[alexander :surkov (:asurkov)]

(In reply to James Teh [:Jamie] from comment #8)
> So nsIDOMXULSelectControlItemElement::GetLabel is not the same as the label
> attribute?

yeah, it's not same as label attribute, autocomplete items have own implementation, see comment #4

(In reply to James Teh [:Jamie] from comment #6)

> nit: As we did previously, it'd be good to have a comment here explaining
> why we exclude select elements; i.e. because the label attribute is used to
> indicate which option is selected.

agreed

(In reply to James Teh [:Jamie] from comment #7)
> Oh damn, I didn't see the last patch. Hang on.

is anything left or should I proceed and land it?

Comment 10

[James Teh [:Jamie]]

(In reply to alexander :surkov (:asurkov) from comment #9)
> yeah, it's not same as label attribute, autocomplete items have own
> implementation, see comment #4

Oh sorry. I must have missed that in the patch storm. :)

> is anything left or should I proceed and land it?

Go ahead. The r+ is now a real r+. :)

Comment 11

[alexander :surkov (:asurkov)]

ok, thanks :)

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/51026c4edd87
stop using nsIDOMXULLabeledControlElement, r=jamie

Comment 13

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/51026c4edd87