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)

defect

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)

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?
OS: Mac OS X → All
Hardware: x86 → All
Version: unspecified → Trunk
Blocks: 407325
I considered exposing nsGlobalWindow::DispatchCustomEvent instead (on nsPIDOMWindow). Please let me know if you would prefer that, ok?
Attachment #543045 - Flags: review?(gavin.sharp)
Attachment #543045 - Flags: review?(Olli.Pettay)
Assignee: nobody → bzbarsky
Priority: -- → P1
Whiteboard: [need review]
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+
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?
I was thinking just testing that the tabbrowser's tab's docShell state is consistent and correct after a chromeWindow.minimize()/focus() etc.
Hmm. Let me try that.
Attached patch Now with test (obsolete) (deleted) — Splinter Review
Attachment #543045 - Attachment is obsolete: true
Attachment #543045 - Flags: review?(Olli.Pettay)
Attachment #543066 - Flags: review?(gavin.sharp)
Attachment #543066 - Flags: review?(Olli.Pettay)
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+
> stick a PD license header on the test too? Done.
Blocks: 668271
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+
Attached patch Updated to comments (deleted) — Splinter Review
Attachment #544886 - Flags: review?(Olli.Pettay)
Attachment #543066 - Attachment is obsolete: true
Attachment #544886 - Flags: review?(Olli.Pettay) → review+
Whiteboard: [need review] → [need landing]
Flags: in-testsuite?
Whiteboard: [need landing]
Target Milestone: --- → Firefox 8
Flags: in-testsuite? → in-testsuite+
backed out due to permaorange in the test, sounds like something needs better merging
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
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.
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.
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.
We don't set sizemode dynamically (except on very small screens during browser startup) as far as I can tell.
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.
> 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
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Depends on: 671160
Depends on: 671635
Need to document the new sizemodechange event, no?
Keywords: dev-doc-needed
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).
Depends on: 1011171
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: