Last Comment Bug 802718 - Manually restore window at end of browser_minimize to avoid breaking rest of suite if test fails
: Manually restore window at end of browser_minimize to avoid breaking rest of ...
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: x86_64 Linux
-- normal (vote)
: Firefox 19
Assigned To: Graeme McCutcheon [:graememcc]
Depends on:
Blocks: 514067
  Show dependency treegraph
Reported: 2012-10-17 11:11 PDT by Graeme McCutcheon [:graememcc]
Modified: 2012-10-18 10:33 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

v1 (842 bytes, patch)
2012-10-17 11:11 PDT, Graeme McCutcheon [:graememcc] review+
Details | Diff | Splinter Review

Description User image Graeme McCutcheon [:graememcc] 2012-10-17 11:11:02 PDT
Created attachment 672402 [details] [diff] [review]

Feel free to WONTFIX if you want, as it doesn't affect the test slaves - it only affects me when trying to run browser-chrome locally...

Unity/Compiz currently has a bug where it doesn't notify windows that they have been minimized. This breaks browser_minimize.js - as expected. However, although the browser-test focuses (and hence raises) the window at the end of each test, the lack of a restore call means our internal widget state continues to report "minimized". This causes all kinds of wackiness and thus test failures. For example, all the PopupNotifications tests fail, as layout won't display popups in a minimized window. Running TEST_PATH=browser/base/content/test make -C obj mochitest-browser-chrome, this causes 41 failures, all fixed by this patch.

Having looked at SetSizeMode in the widget code for the various platforms (which is where we end up during window.restore, it's essentially a NOP when the window is already restored.

I considered also throwing this in waitForWindowState in the harness, but wasn't convinced it was really necessary for 1 test running under 1 particular window manager.
Comment 1 User image :Gavin Sharp [email:] 2012-10-17 11:44:45 PDT
Comment on attachment 672402 [details] [diff] [review]

seems fine to me - one nit: put the call to registerCleanupFunction in test() (it's generally a bad idea for tests to run code outside of their test() function).
Comment 2 User image Graeme McCutcheon [:graememcc] 2012-10-18 05:05:52 PDT
Comment 3 User image Ed Morley [:emorley] 2012-10-18 10:33:01 PDT

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