378171 - Offscreen should always be true for hidden elements

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Aaron Leventhal]

It is not sensible to try to calculate offscreen for hidden elements.

In fact, the ATK code already explicitly sets it.
We should do that in cross-platform code.

In fact this is causing the recursion in bug 377936.

Comment 1

[Aaron Leventhal]

Attachment: Fix IsVisible()

Comment 2

[alexander :surkov (:asurkov)]

Comment on attachment 262245 [details] [diff] [review]
Fix IsVisible()

looks ok

Comment 4

[alexander :surkov (:asurkov)]

Aaron, it looks patch isn't correct totally.

Now we can have invisible=true and offscreen=false (because initial value of offscreen is false) and it means atk will have visible=false and showing=true, that is not correct.

Comment 5

[Aaron Leventhal]

Attachment: How does this look?

Comment 6

[alexander :surkov (:asurkov)]

Comment on attachment 262276 [details] [diff] [review]
How does this look?


>+  if (rectVisibility == nsRectVisibility_kZeroAreaRect || !mDOMNode) {
>+    *aIsOffscreen = PR_TRUE; // Offscreen always set to true for hidden elements

You don't need to do *aIsOffscreen = PR_TRUE

>+    return PR_FALSE;
>+  }
>+  
>+  // Currently one of:
>+  // nsRectVisibility_kVisible, 
>+  // nsRectVisibility_kAboveViewport, 
>+  // nsRectVisibility_kBelowViewport, 
>+  // nsRectVisibility_kLeftOfViewport, 
>+  // nsRectVisibility_kRightOfViewport
>+  // This view says it is visible, but we need to check the parent view chain :(
>+  nsCOMPtr<nsIDOMDocument> domDoc;
>+  mDOMNode->GetOwnerDocument(getter_AddRefs(domDoc));
>+  NS_ENSURE_TRUE(domDoc, NS_ERROR_FAILURE);

I would prefer to return PR_TRUE/PR_FALSE explicetly.

> 
>-    nsCOMPtr<nsIDocument> doc(do_QueryInterface(domDoc));
>-    NS_ENSURE_TRUE(doc, NS_ERROR_FAILURE);
>+  nsCOMPtr<nsIDocument> doc(do_QueryInterface(domDoc));
>+  NS_ENSURE_TRUE(doc, NS_ERROR_FAILURE);

here too

Comment 7

[Aaron Leventhal]

Attachment: Address Surkov's comments