Closed
Bug 648045
Opened 14 years ago
Closed 13 years ago
Mark the active tab in minimized windows as inactive
Categories
(Firefox :: General, defect, P1)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 8
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
(Depends on 1 open bug)
Details
(Keywords: dev-doc-complete)
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
We do all sorts of aggressive throttling for background tabs, but we define "background" as "not currently selected in the tabbrowser". We should consider extending this definition to all tabs in minimized windows.
Gavin, whom should I cc or where should I put this bug to get some traction?
I guess this and bug 648046 should be platform-independent?
Assignee | ||
Comment 2•13 years ago
|
||
I considered exposing nsGlobalWindow::DispatchCustomEvent instead (on nsPIDOMWindow). Please let me know if you would prefer that, ok?
Assignee | ||
Updated•13 years ago
|
Attachment #543045 -
Flags: review?(gavin.sharp)
Attachment #543045 -
Flags: review?(Olli.Pettay)
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → bzbarsky
Priority: -- → P1
Whiteboard: [need review]
Comment 3•13 years ago
|
||
Comment on attachment 543045 [details] [diff] [review]
Mark the currently selected tab in a minimized window as inactive.
Should be pretty easy to add a test, right?
Attachment #543045 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 4•13 years ago
|
||
I actually have no idea how to add a test for this... which part of it would be testable? That the event is fired on minimize? Or that the docshell is set to not be active on minimize? How can I even trigger minimize in a test?
Comment 5•13 years ago
|
||
I was thinking just testing that the tabbrowser's tab's docShell state is consistent and correct after a chromeWindow.minimize()/focus() etc.
Assignee | ||
Comment 6•13 years ago
|
||
Hmm. Let me try that.
Assignee | ||
Comment 7•13 years ago
|
||
Assignee | ||
Updated•13 years ago
|
Attachment #543045 -
Attachment is obsolete: true
Attachment #543045 -
Flags: review?(Olli.Pettay)
Assignee | ||
Updated•13 years ago
|
Attachment #543066 -
Flags: review?(gavin.sharp)
Attachment #543066 -
Flags: review?(Olli.Pettay)
Comment 8•13 years ago
|
||
Comment on attachment 543066 [details] [diff] [review]
Now with test
stick a PD license header on the test too?
https://www.mozilla.org/MPL/boilerplate-1.1/pd-c
Attachment #543066 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 9•13 years ago
|
||
> stick a PD license header on the test too?
Done.
Comment 10•13 years ago
|
||
Comment on attachment 543066 [details] [diff] [review]
Now with test
nsPIDOMWindow::DispatchCustomEvent could be useful also in other cases.
So r+ either way.
Attachment #543066 -
Flags: review?(Olli.Pettay) → review+
Assignee | ||
Comment 11•13 years ago
|
||
Attachment #544886 -
Flags: review?(Olli.Pettay)
Assignee | ||
Updated•13 years ago
|
Attachment #543066 -
Attachment is obsolete: true
Updated•13 years ago
|
Attachment #544886 -
Flags: review?(Olli.Pettay) → review+
Assignee | ||
Updated•13 years ago
|
Whiteboard: [need review] → [need landing]
Assignee | ||
Comment 12•13 years ago
|
||
Flags: in-testsuite?
Whiteboard: [need landing]
Target Milestone: --- → Firefox 8
Assignee | ||
Comment 13•13 years ago
|
||
And also http://hg.mozilla.org/integration/mozilla-inbound/rev/9e4ab3907b29 to merge the test...
Flags: in-testsuite? → in-testsuite+
Comment 14•13 years ago
|
||
backed out due to permaorange in the test, sounds like something needs better merging
Comment 15•13 years ago
|
||
this is after the push in comment 13:
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_minimize.js | Docshell should be active - Got undefined, expected true
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_minimize.js | Docshell should be inactive - Got undefined, expected false
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_minimize.js | Docshell should be active again - Got undefined, expected true
Assignee | ||
Comment 16•13 years ago
|
||
OK, the second changeset is not really needed, since that's working with tabbrowser, not browser.
The first changeset passes tests on Mac and Windows, but on Linux unminimizing seems to not mark as active? Testing that now.
Assignee | ||
Comment 17•13 years ago
|
||
OK, on Linux minimize/restore are very async and sort of not in sync with window state changes. I'll rewrite the test to handle that.
Comment 18•13 years ago
|
||
I'm not sure about this, but the sizemode attribute on the document element (if it even gets set) may be more in sync with window state changes than windowState.
Assignee | ||
Comment 19•13 years ago
|
||
We don't set sizemode dynamically (except on very small screens during browser startup) as far as I can tell.
Comment 20•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/annotate/dd9a2ec82f68/dom/tests/mochitest/chrome/sizemode_attribute.xul
tests dynamic sizemode changes on WINNT.
It is apparently broken on Mac (bug 409095).
There seems to be some dynamic change happening on X11. sizemode gets set to maximized at least, but part of the test fails, perhaps due to timing (fullscreen is not the correct event to wait for) or perhaps due to incomplete implementation.
Assignee | ||
Comment 21•13 years ago
|
||
> tests dynamic sizemode changes on WINNT.
Ah, interesting. Looks like we do update it in some cases (e.g. nsXULWindow::SavePersistentAttributes), but we don't do that for minimized sizemode.
In any case, I just made the test poll for the right docshell active state; if that fails to get set the test will time out.
http://hg.mozilla.org/integration/mozilla-inbound/rev/5decde435e4a
Comment 22•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 23•13 years ago
|
||
Need to document the new sizemodechange event, no?
Keywords: dev-doc-needed
Comment 24•13 years ago
|
||
Updated documentation:
https://developer.mozilla.org/en/Gecko/Gecko_event_reference
https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIDocShell
Also updated Firefox 8 for developers.
Keywords: dev-doc-needed → dev-doc-complete
Comment 25•13 years ago
|
||
I moved the docs for the sizemodechange event to https://developer.mozilla.org/en/XUL/Events#Window_events , since that's where other top-level-window events were documented (even though it probably works with HTML-as-the-top-level-doc).
Note that the event is fired way too often at least on Mac (bug 715867).
You need to log in
before you can comment on or make changes to this bug.
Description
•