604329 - When display goes off active state of the topmost tab should be set to false.

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: General, defect)

Description

[Jon Hemming]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.11) Gecko/2009060309 Ubuntu/8.04 (hardy) Firefox/3.0.11
Build Identifier: 

mIsActive in nsDocShell is set to false when tab goes to background. This way
all other tabs than topmost tab has mIsActive==false. mIsActive of the topmost
tab should also be set to false when display goes off since it is not
active anymore.

Reproducible: Always

Comment 1

[Jon Hemming]

Attachment: Sets isActive of docShell of the topmost tab to false when display goes off.

Could this be done on fennec side like this or should it be handled somewhere else?

Comment 2

[Jon Hemming]

Attachment: Observes correct notification and sets tab also to inactive when browser goes to background

This would also fix bug 603286 for fennec.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 483436 [details] [diff] [review]
Observes correct notification and sets tab also to inactive when browser goes to background

Some feedback:
* Let's just use a singleton, like the other observers.
* BrowserActivityStateObserver is a great name
* getTopWindow will always be true for browser.xul, we can probably skip it and just hookup the events
* Browser:SetTabToActive and Browser:SetTabToInactive are very close to Browser:Focus and Browser:Blur - can we safely reuse?
* If we can't reuse, change names to Browser:Activate and Browser:Deactivate. Also structure the switch statement so that the messages can share common code?

>+    if (Browser._selectedTab) {
>+      if (Browser._selectedTab._browser) {

Change to:
 if (Browser.selectedBrowser) {

>+        if (Browser._selectedTab._browser.messageManager) {

This will always be true in Fennec

Watch the "if("

Overall looks good

Comment 4

[Jon Hemming]

Attachment: Version 2

Messages to content side changed to "Browser:Activate" and "Browser:Inactivate" since didn't find a good way to do those with "Browser:Focus" and "Browser:Blur". It could have been done with using json argument, but I think that would led to more complicated/confusing code. Also I don't know any good way to make the code more compact on content side, so I left that as it is.

Comment 5

[Jon Hemming]

(In reply to comment #3)
> * Let's just use a singleton, like the other observers.
done

> * getTopWindow will always be true for browser.xul, we can probably skip it and
> just hookup the events
getTopWindow function dropped.

> * Browser:SetTabToActive and Browser:SetTabToInactive are very close to
> Browser:Focus and Browser:Blur - can we safely reuse?
Didn't find a good way. Content can't be used to keep track of active state of the browser, which could have been one way to reuse, and using json argument would have led to more complicated code.

> * If we can't reuse, change names to Browser:Activate and Browser:Deactivate.
> Also structure the switch statement so that the messages can share common code?
Didn't come up with any good way.

> >+    if (Browser._selectedTab) {
> >+      if (Browser._selectedTab._browser) {
> 
> Change to:
>  if (Browser.selectedBrowser) {
> 
> >+        if (Browser._selectedTab._browser.messageManager) {
> 
> This will always be true in Fennec
> 
Done.

Comment 6

[BMO Automation]

Closing all opened bug in a graveyard component