393040 - GetName() fails on menupopup

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 1

[Aaron Leventhal]

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

> +  nsCOMPtr<nsIContent> content(do_QueryInterface(mDOMNode));

I don't think you can do that with a COMPtr. Some compilers can't deal with that.
You can do it with nsIContent* or by using a temp variable and then doing tempParent.swap(content);

Please find out before checking in.

Comment 2

[alexander :surkov (:asurkov)]

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

(In reply to comment #1)
> Please find out before checking in.
> 

It's safe to ask Neil :)

Comment 3

[[e-mail]]

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

>-    nsCOMPtr<nsIDOMNode> parentNode, node(do_QueryInterface(element));
Actually the old code was worse, that node was unnecessary (because you can call GetParentNode on element).

>+  nsCOMPtr<nsIContent> content(do_QueryInterface(mDOMNode));
>+  while (content && aName.IsEmpty()) {
>+    content->GetAttr(kNameSpaceID_None, nsAccessibilityAtoms::label, aName);
>+    content = content->GetParent();
>   }
>+  return NS_OK;
I notice that the old code failed if it found no label, whereas the new code only fails if its node is null. Do you really want this change? If you don't, then you can also remove the null node check, as then you'll fail by not finding a label.

Comment 4

[alexander :surkov (:asurkov)]

(In reply to comment #3)

> I notice that the old code failed if it found no label, whereas the new code
> only fails if its node is null. Do you really want this change? If you don't,
> then you can also remove the null node check, as then you'll fail by not
> finding a label.
> 

If I got you right then proposed behaviour is bug is about. The method should fail only if mDOMNode is null (that means accessible is out of date). For another cases we shouldn't fail. The main reason is to fail is not good for usage from js.

Comment 5

[[e-mail]]

(In reply to comment #4)
>If I got you right then proposed behaviour is bug is about.
Ah, I did wonder :-)

Comment 6

[alexander :surkov (:asurkov)]

checked in