Closed
Bug 604329
Opened 14 years ago
Closed 6 years ago
When display goes off active state of the topmost tab should be set to false.
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: jon.hemming, Unassigned)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
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
Reporter | ||
Updated•14 years ago
|
Reporter | ||
Comment 1•14 years ago
|
||
Could this be done on fennec side like this or should it be handled somewhere else?
Reporter | ||
Updated•14 years ago
|
Attachment #483128 -
Flags: feedback?(mark.finkle)
Reporter | ||
Comment 2•14 years ago
|
||
This would also fix bug 603286 for fennec.
Attachment #483128 -
Attachment is obsolete: true
Attachment #483436 -
Flags: feedback?(mark.finkle)
Attachment #483128 -
Flags: feedback?(mark.finkle)
Comment 3•14 years ago
|
||
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
Attachment #483436 -
Flags: feedback?(mark.finkle) → feedback+
Reporter | ||
Comment 4•14 years ago
|
||
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.
Attachment #483436 -
Attachment is obsolete: true
Attachment #485034 -
Flags: review?(mark.finkle)
Reporter | ||
Comment 5•14 years ago
|
||
(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.
Updated•14 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: in-testsuite?
Updated•12 years ago
|
Attachment #485034 -
Flags: review?(mark.finkle)
Comment 6•6 years ago
|
||
Closing all opened bug in a graveyard component
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•