600529 - chrome://mochikit/content/browser/mobile/chrome/browser_mainui.js | Content container is same height as window

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

this morning I did a hg pull on m-c/m-b and a fresh build then ran the browser-chrome tests.  I found that browser_mainui.js has 1 failure:

TEST-START | chrome://mochikit/content/browser/mobile/chrome/browser_mainui.js
TEST-PASS | chrome://mochikit/content/browser/mobile/chrome/browser_mainui.js | Main window should be browser.xul
TEST-PASS | chrome://mochikit/content/browser/mobile/chrome/browser_mainui.js | Should have a browser
TEST-PASS | chrome://mochikit/content/browser/mobile/chrome/browser_mainui.js | selectedBrowser == selectedTab.browser
TEST-PASS | chrome://mochikit/content/browser/mobile/chrome/browser_mainui.js | Content container is same width as window
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/mobile/chrome/browser_mainui.js | Content container is same height as window
TEST-INFO | chrome://mochikit/content/browser/mobile/chrome/browser_mainui.js | Test took 0.001s to complete

TEST-INFO | checking window state
TEST-INFO | chrome://mochikit/content/browser/mobile/chrome/browser_mainui.js | Console message: [JavaScript Warning: "reference to undefined property window.gBrowser" {file: "chrome://mochikit/content/browser-test.js" line: 82}]


Here is the offending line of code:
http://mxr.mozilla.org/mobile-browser/source/chrome/tests/browser_mainui.js#26

Comment 1

[Matt Brubeck (:mbrubeck)]

When I change that from an "ok" test to an "is" test, I get:

Content container is same height as window - Got 486, expected 480

Comment 2

[Matt Brubeck (:mbrubeck)]

Testing manually, when there is only one tab open then #content-viewport has the correct height.  As soon as a second tab is opened, then #browsers and everything containing it is 6px too tall, although the actual <browser> elements have the correct height.  This is still the case even if the second tab is then closed.

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

sounds like the test found a bug

Comment 4

[Matt Brubeck (:mbrubeck)]

Attachment: patch

This makes the test pass.  This might also be related to bug 597530, although this patch doesn't fix that bug.

The important change is in browser.xul; the other changes are just cleanup.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 479492 [details] [diff] [review]
patch


>diff -r 77eed4a3ac43 chrome/content/browser.xul
>-              <stack id="tile-stack" class="window-width" flex="1">
>+              <stack id="tile-stack" class="window-width widow-height" flex="1">

window-height, not widow-height

I was going to ask if this was the only "window-height" that was required, but I guess it is not. If the test is fixed and this was not needed, lets just remove the "window-height" from the <stack>

Comment 6

[Matt Brubeck (:mbrubeck)]

pushed with unnecessary/misspelled changes removed:
http://hg.mozilla.org/mobile-browser/rev/c72c98928525

Comment 7

[Cristian Nicolae (:xti)]

I've performed a manual testing following Matt's steps from comment #2 and there weren't any errors in console related to this bug. I'll mark it as verified fixed.

--
Mozilla/5.0 (Android;Linux armv7l;rv:9.0a1)Gecko/20110912
Firefox/9.0a1 Fennec/9.0a1
Device: Samsung Galaxy S
OS: Android 2.2