1368103 - Accessibles in background tabs don't have offscreen state in e10s

Status: RESOLVED FIXED

(Core :: Disability Access APIs, enhancement)

Description

[Eitan Isaacson [:eeejay]]

Before e10s, every accessible in a non-visible tab would have an offscreen state.

Comment 1

[Eitan Isaacson [:eeejay]]

Attachment: Use DOM doc visibilityState to determine if a tab is hidden. r?surkov

Comment 2

[alexander :surkov (:asurkov)]

Comment on attachment 8871963 [details] [diff] [review]
Use DOM doc visibilityState to determine if a tab is hidden. r?surkov

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

I trust you that IsHidden is an equivalent in this case but where it is defined?

::: accessible/generic/Accessible.cpp
@@ +345,5 @@
>        return 0;
>  
> +    // Offscreen state if the document's visibility state is not visible.
> +    if (!Document() || Document()->IsHidden())
> +      return states::OFFSCREEN;

no Document() check is required I believe, since no one should get here for shutdown accessible

::: accessible/generic/DocAccessible.h
@@ +138,5 @@
>    }
>  
> +  bool IsHidden() const
> +  {
> +    return !mDocumentNode || mDocumentNode->Hidden();

same here, you can safely skip !mDocumentNode check I think

Comment 3

[Eitan Isaacson [:eeejay]]

I am nervously removing those null-checks!

Comment 4

[Eitan Isaacson [:eeejay]]

Attachment: Use DOM doc visibilityState to determine if a tab is hidden. r?surkov

I needed to re-add the xul:tabpanel checks for cases where the tabs are not documents.
Otherwise test_visibility.xul fails. Having both checks will accomodate both cases.

Comment 5

[alexander :surkov (:asurkov)]

(In reply to Eitan Isaacson [:eeejay] from comment #3)
> I am nervously removing those null-checks!

you never should get here, if they fail, then something very bad happened

(In reply to Eitan Isaacson [:eeejay] from comment #4)
> Created attachment 8873885 [details] [diff] [review]
> Use DOM doc visibilityState to determine if a tab is hidden. r?surkov
> 
> I needed to re-add the xul:tabpanel checks for cases where the tabs are not
> documents.
> Otherwise test_visibility.xul fails. Having both checks will accomodate both
> cases.

thanks we have a test, otherwise Marco would be surprised by not working prefs and other UI stuff I guess :)

Comment 6

[alexander :surkov (:asurkov)]

Comment on attachment 8873885 [details] [diff] [review]
Use DOM doc visibilityState to determine if a tab is hidden. r?surkov

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

::: accessible/generic/Accessible.cpp
@@ +345,5 @@
>        return 0;
>  
> +    // Offscreen state if the document's visibility state is not visible.
> +    if (Document()->IsHidden())
> +      return states::OFFSCREEN;

shouldn't we move this out the cycle? when Document::IsHidden() returns true? Is it always about background tab documents?

Comment 7

[Eitan Isaacson [:eeejay]]

(In reply to alexander :surkov from comment #6)
> Comment on attachment 8873885 [details] [diff] [review]
> Use DOM doc visibilityState to determine if a tab is hidden. r?surkov
> 
> Review of attachment 8873885 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/generic/Accessible.cpp
> @@ +345,5 @@
> >        return 0;
> >  
> > +    // Offscreen state if the document's visibility state is not visible.
> > +    if (Document()->IsHidden())
> > +      return states::OFFSCREEN;
> 
> shouldn't we move this out the cycle? when Document::IsHidden() returns
> true?

Good point!

> Is it always about background tab documents?

When the document is not visible (like if the window is minimized too). You can test it with document.visibilityState

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6f99018495e7
Use DOM doc visibilityState to determine if a tab is hidden. r=surkov

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/6f99018495e7