Last Comment Bug 648045 - Mark the active tab in minimized windows as inactive
: Mark the active tab in minimized windows as inactive
Status: RESOLVED FIXED
: dev-doc-complete
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
: P1 normal with 2 votes (vote)
: Firefox 8
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
:
:
Mentors:
Depends on: 1011171 671160 671635
Blocks: 407325 668271 684805
  Show dependency treegraph
 
Reported: 2011-04-06 11:12 PDT by Boris Zbarsky [:bz] (still a bit busy)
Modified: 2014-05-15 12:31 PDT (History)
16 users (show)
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Mark the currently selected tab in a minimized window as inactive. (6.14 KB, patch)
2011-06-29 20:33 PDT, Boris Zbarsky [:bz] (still a bit busy)
gavin.sharp: review+
Details | Diff | Splinter Review
Now with test (7.40 KB, patch)
2011-06-29 22:33 PDT, Boris Zbarsky [:bz] (still a bit busy)
gavin.sharp: review+
bugs: review+
Details | Diff | Splinter Review
Updated to comments (10.38 KB, patch)
2011-07-08 13:12 PDT, Boris Zbarsky [:bz] (still a bit busy)
bugs: review+
Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] (still a bit busy) 2011-04-06 11:12:28 PDT
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?
Comment 1 [Baboo] 2011-04-10 04:40:14 PDT
I guess this and bug 648046 should be platform-independent?
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2011-06-29 20:33:25 PDT
Created attachment 543045 [details] [diff] [review]
Mark the currently selected tab in a minimized window as inactive.

I considered exposing nsGlobalWindow::DispatchCustomEvent instead (on nsPIDOMWindow).  Please let me know if you would prefer that, ok?
Comment 3 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-06-29 21:44:42 PDT
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?
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2011-06-29 22:13:14 PDT
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 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-06-29 22:16:21 PDT
I was thinking just testing that the tabbrowser's tab's docShell state is consistent and correct after a chromeWindow.minimize()/focus() etc.
Comment 6 Boris Zbarsky [:bz] (still a bit busy) 2011-06-29 22:26:21 PDT
Hmm.  Let me try that.
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2011-06-29 22:33:33 PDT
Created attachment 543066 [details] [diff] [review]
Now with test
Comment 8 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-06-29 22:50:28 PDT
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
Comment 9 Boris Zbarsky [:bz] (still a bit busy) 2011-06-29 22:55:19 PDT
> stick a PD license header on the test too?

Done.
Comment 10 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2011-07-08 08:05:26 PDT
Comment on attachment 543066 [details] [diff] [review]
Now with test

nsPIDOMWindow::DispatchCustomEvent could be useful also in other cases.

So r+ either way.
Comment 11 Boris Zbarsky [:bz] (still a bit busy) 2011-07-08 13:12:52 PDT
Created attachment 544886 [details] [diff] [review]
Updated to comments
Comment 12 Boris Zbarsky [:bz] (still a bit busy) 2011-07-11 06:16:40 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/3abc0dbbf710
Comment 13 Boris Zbarsky [:bz] (still a bit busy) 2011-07-11 06:21:09 PDT
And also http://hg.mozilla.org/integration/mozilla-inbound/rev/9e4ab3907b29 to merge the test...
Comment 14 Marco Bonardo [::mak] 2011-07-11 10:31:25 PDT
backed out due to permaorange in the test, sounds like something needs better merging
Comment 15 Marco Bonardo [::mak] 2011-07-11 10:32:07 PDT
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
Comment 16 Boris Zbarsky [:bz] (still a bit busy) 2011-07-11 11:29:31 PDT
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.
Comment 17 Boris Zbarsky [:bz] (still a bit busy) 2011-07-11 12:07:52 PDT
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 Karl Tomlinson (:karlt) 2011-07-11 15:38:16 PDT
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.
Comment 19 Boris Zbarsky [:bz] (still a bit busy) 2011-07-11 18:56:26 PDT
We don't set sizemode dynamically (except on very small screens during browser startup) as far as I can tell.
Comment 20 Karl Tomlinson (:karlt) 2011-07-11 19:02:06 PDT
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.
Comment 21 Boris Zbarsky [:bz] (still a bit busy) 2011-07-11 19:37:07 PDT
> 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 Mounir Lamouri (:mounir) 2011-07-12 03:48:47 PDT
Merged:
http://hg.mozilla.org/mozilla-central/rev/5decde435e4a
Comment 23 neil@parkwaycc.co.uk 2011-09-06 01:39:04 PDT
Need to document the new sizemodechange event, no?
Comment 24 Eric Shepherd [:sheppy] 2011-09-06 06:22:11 PDT
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.
Comment 25 Nickolay_Ponomarev 2012-01-06 05:41:50 PST
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).

Note You need to log in before you can comment on or make changes to this bug.