Eitan Isaacson [:eeejay] Assignee
	Description • 7 years ago

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

	alexander :surkov (:asurkov)
	Comment 2 • 7 years ago

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

	Eitan Isaacson [:eeejay] Assignee
	Comment 3 • 7 years ago

I am nervously removing those null-checks!

	Eitan Isaacson [:eeejay] Assignee
	Comment 4 • 7 years ago

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.

	alexander :surkov (:asurkov)
	Comment 5 • 7 years ago

(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 :)

	alexander :surkov (:asurkov)
	Comment 6 • 7 years ago

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?

	Eitan Isaacson [:eeejay] Assignee
	Comment 7 • 7 years ago

(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

	Pulsebot
	Comment 8 • 7 years ago

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

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 9 • 7 years ago
bugherder

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