370455 - SHOWING state incorrectly set for content of tabs whose content is not showing

Status: RESOLVED FIXED

(Firefox :: Disability Access, defect)

Description

[Willie Walker]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a3pre) Gecko/20070212 Minefield/3.0a3pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a3pre) Gecko/20070212 Minefield/3.0a3pre

If I open two URLs in separate tabs in Firefox, the content of tabs that are not showing still seem to have the SHOWING state set.

Reproducible: Always

Steps to Reproduce:
1. Run Firefox 
2. Open http://www.google.com in one tab
3. Open http://www.mozilla.org in another
4. Select the mozilla tab, hiding the google tab
5. Analyze the accessible hierarchy using at-poke
Actual Results:  
You'll see that the elements in the google tab appear with the SHOWING state

Expected Results:  
Content that is not on the screen should not have the SHOWING state set.

Comment 1

[Evan Yan]

confirmed.

Comment 2

[Nian Liu(n/a in a long time)]

1. SHOWING is mapped from OFFSCREEN which is only meaningful when VISIBLE is set. so we either 
a)suggest at-tool to use SHOWING only when VISIBLE is avaliable or
b)give SHOWING a better map. however, this will break state map rule and i haven't see an easy way.

2.view's chain is broken at document frame. that's why outDocFrame is hide while content frame is visible. see 130078

Comment 3

[Nian Liu(n/a in a long time)]

(In reply to comment #2)
> 1. SHOWING is mapped from OFFSCREEN which is only meaningful when VISIBLE is
> set. so we either 
> a)suggest at-tool to use SHOWING only when VISIBLE is avaliable or
> b)give SHOWING a better map. however, this will break state map rule and i
> haven't see an easy way.
or just crack it after translateState?

Comment 4

[Willie Walker]

I think the best thing to do here is to adhere to the AT-SPI spec and then, when in doubt, refer to the reference implementation of GTK+/gail.  Here's the text from the at-spi idl spec:

  /**
   * Indicates this object, the object's parent, the object's parent's
   * parent, and so on, are all 'shown' to the end-user, i.e.
   * subject to "exposure" if blocking or obscuring objects do not interpose
   * between this object and the top of the window stack.
   */
  STATE_SHOWING

SHOWING should not be present if the thing isn't showing.  Thanks!

Comment 5

[Aaron Leventhal]

It won't hurt anything if OFFSCREEN is not exposed when something is not VISIBLE.

Comment 6

[Nian Liu(n/a in a long time)]

Attachment: test case

iframe should also be solved.

Comment 7

[Nian Liu(n/a in a long time)]

Attachment: rough work

half away being done

Comment 8

[Nian Liu(n/a in a long time)]

Attachment: patch

Comment 9

[alexander :surkov (:asurkov)]

Why can't nsDocAccessible use nsAccessible::IsVisible()? What's wrong in logic of IsVisible for document?

Comment 10

[Nian Liu(n/a in a long time)]

(In reply to comment #9)
> Why can't nsDocAccessible use nsAccessible::IsVisible()? What's wrong in logic
> of IsVisible for document?
> 

i guess nsDocAccessible doesn't care of STATE_OFFSCREEN?
view chain is broken between chrome and content. chrome may invisible while you get content is visible currentlly.

Comment 11

[alexander :surkov (:asurkov)]

(In reply to comment #10)
> (In reply to comment #9)
> > Why can't nsDocAccessible use nsAccessible::IsVisible()? What's wrong in logic
> > of IsVisible for document?
> > 
> 
> i guess nsDocAccessible doesn't care of STATE_OFFSCREEN?

But note you modify nsDocAccessible::getState, that calls nsAccessible::getState that calls IsVisible(). So nsDocAcc's state can have STATE_INVISIBLE and STATE_OFFSCREEN. It means STATE_VISIBLE is not setted for doc accessible therefore I wonder what's wrong in IsVisible() logic?

Comment 12

[Nian Liu(n/a in a long time)]

nsAccessible::GetState will return NS_ERROR_FAILURE if it's not have a content.  see http://mxr.mozilla.org/seamonkey/source/accessible/src/base/nsAccessible.cpp#978

document is the scenerio that doesn't have a content

Comment 13

[Aaron Leventhal]

We should probably just fail if there is no mDOMNode.

If there's no content, don't use nsIContent to get information, but it doesn't necessarily mean we should fail.

Comment 14

[alexander :surkov (:asurkov)]

(In reply to comment #13)
> We should probably just fail if there is no mDOMNode.
> 
> If there's no content, don't use nsIContent to get information, but it doesn't
> necessarily mean we should fail.
> 

otherwise we should remove nsAccessible::getState call from nsDocAccessible.

Is relation 1-1 between frame with view and widget? What is frame with view and what is widget?

Comment 15

[Aaron Leventhal]

A subset of frames have a view.
A subset of views have a widget. Each widget is for an OS window (HWND on Win32).

Comment 16

[alexander :surkov (:asurkov)]

Ok, it's not clear ATK changes for me. Please say me why they are needed?

Comment 17

[Nian Liu(n/a in a long time)]

SHOWING means nothing when accessible object is not VISIBLE. 

Comment 18

[alexander :surkov (:asurkov)]

(In reply to comment #17)
> SHOWING means nothing when accessible object is not VISIBLE. 
> 

Ok. Atk SHOWING <-> Gecko OFFSCREEN. Why do you should unset it, if it should happend when offscreen state is (un)setted up?

Comment 19

[alexander :surkov (:asurkov)]

(In reply to comment #18)
> (In reply to comment #17)
> > SHOWING means nothing when accessible object is not VISIBLE. 
> > 
> 
> Ok. Atk SHOWING <-> Gecko OFFSCREEN. Why do you should unset it, if it should
> happend when offscreen state is (un)setted up?
> 

I mean it looks you don't need it because TranslateStates should do it. Why doesn't TranslateStates do it?

Comment 20

[alexander :surkov (:asurkov)]

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


>+  PRBool CheckVisibilityInParentChain(nsIDocument* aDocument, nsIView* aView);

Please document it. Though the name is very good but it's fine to have some comment.

>+PRBool nsAccessible::CheckVisibilityInParentChain(nsIDocument* aDocument, nsIView* aView)
>+{
>+  // both view chain and widget chain are broken between chrome and content
>+  while (aDocument != nsnull) {
>+    while (aView != nsnull) {
>+      if (aView->GetVisibility() == nsViewVisibility_kHide) {
>+        return PR_FALSE;
>+      }
>+      aView = aView->GetParent();

I'd prefer to use local variable. It's not very fine with code readable when input arguement is changed.

The same for aDocument.

> 
>     TranslateStates(accExtState, gAtkStateMapExt, state_set);
> 
>+    if (!atk_state_set_contains_state(state_set, ATK_STATE_VISIBLE)) {
>+      atk_state_set_remove_state(state_set, ATK_STATE_SHOWING);
>+    }

Please add good comment why do we need this. Like you said on irc you should say that gecko OFFSCREEN and INVISIBLE lives together, but SHOWING and not VISIBLE do not.

Comment 21

[Nian Liu(n/a in a long time)]

Attachment: patch to be checked in

addressing surkov's comment

Comment 22

[Ginn Chen]

committed in.
Add null check for mDOMNode
Changed STATE_INVISIBLE to nsIAccessibleStates::STATE_INVISIBLE