1087303 - Add test for the functionality of remaining Panel Menu buttons in Australis (fullscreen, preferences)

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Mihaela Velimiroviciu (:mihaelav)]

This is a follow up of bug 947914, but for the remaining buttons: preferences, fullscreen and new 10s window.

Steps:
1. Start Firefox
2. Open the Panel Menu
3. Click on "New E10 Window", "Fullscreen" and "Preferences" buttons in the Panel Menu

Expected result:
Each button should perform the right corresponding action

Comment 1

[Mihaela Velimiroviciu (:mihaelav)]

When I'm trying to access the "New E10s window" button from a browser-chrome test the button is not found, although it exists there if I manually check it (for the same build). Can someone give a hint on why I can't access that button in the test, please?
Thanks!

Comment 2

[Mike Conley (:mconley) (:⚙️)]

(In reply to Mihaela Velimiroviciu [QA] (:mihaelav) from comment #1)
> When I'm trying to access the "New E10s window" button from a browser-chrome
> test the button is not found, although it exists there if I manually check
> it (for the same build). Can someone give a hint on why I can't access that
> button in the test, please?
> Thanks!

Hey Mihaela - I actually recommend not testing the New E10s window button, since this button is not supposed to make it to release. It's temporary while we do nightly testing of E10s.

Comment 3

[Mihaela Velimiroviciu (:mihaelav)]

Attachment: v1

Tests for fullscreen and preferences buttons

Comment 4

[:Gijs (he/him)]

Comment on attachment 8512669 [details] [diff] [review]
v1

Review of attachment 8512669 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for pursuing this!

::: browser/components/customizableui/test/browser_1087303_button_fullscreen.js
@@ +1,3 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> +  * License, v. 2.0. If a copy of the MPL was not distributed with this
> +  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

Nit: unless you mind licensing this public domain, could you get rid of the license header?

@@ +13,5 @@
> +
> +  let fullscreenButton = document.getElementById("fullscreen-button");
> +  ok(fullscreenButton, "Fullscreen button appears in Panel Menu");
> +
> +  if(isOSX) {

Can we please skip-if = os == "mac" in browser.ini instead? :-)

::: browser/components/customizableui/test/browser_1087303_button_preferences.js
@@ +1,3 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public
> +  * License, v. 2.0. If a copy of the MPL was not distributed with this
> +  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

Nit: unless you mind licensing this public domain, can you get rid of the license header?

@@ +13,5 @@
> +add_task(function() {
> +  info("Check preferences button existence and functionality");
> +
> +  initialPrefValue = Services.prefs.getBoolPref(PREF_INCONTENT);
> +  Services.prefs.setBoolPref(PREF_INCONTENT, "true");

no quotes for true

@@ +26,5 @@
> +  newTab = gBrowser.selectedTab;
> +  yield waitForPageLoad(newTab);
> +
> +  let prefsPage = gBrowser.selectedBrowser.contentWindow.document.
> +                            getElementById("categories");

This will fail on e10s. I'm enabling these tests in e10s mode by default in bug 1080787. Please test this some other way, like by checking the selectedBrowser's currentURI. :-)

@@ +31,5 @@
> +  ok(prefsPage, "Preferences page was opened");
> +});
> +
> +add_task(function asyncCleanup() {
> +  gBrowser.addTab(initialLocation);

Why do you need to add a new tab?

@@ +36,5 @@
> +  gBrowser.removeTab(gBrowser.selectedTab);
> +  info("Tabs were restored");
> +
> +  // restore the browser.preferences.inContent preference
> +  Services.prefs.setBoolPref(PREF_INCONTENT, initialPrefValue);

No need to save the value, use Services.prefs.clearUserPref(PREF_INCONTENT) instead.

@@ +44,5 @@
> +  let deferred = Promise.defer();
> +
> +  let timeoutId = setTimeout(() => {
> +    aTab.linkedBrowser.removeEventListener("load", onTabLoad, true);
> +    deferred.reject("TabSelect did not happen within " + 20000 + "ms");

You're waiting for load, but this says tabselect? That doesn't sound right...

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Just a drive-by note while looking through try for unrelated reasons:
The bug number in this patch is incorrect, and should be fixed before this lands.

Comment 6

[Mihaela Velimiroviciu (:mihaelav)]

Attachment: v1.1

(In reply to :Gijs Kruitbosch from comment #4)
> @@ +31,5 @@
> > +  ok(prefsPage, "Preferences page was opened");
> > +});
> > +
> > +add_task(function asyncCleanup() {
> > +  gBrowser.addTab(initialLocation);
> 
> Why do you need to add a new tab?

If only one (about:blank) tab is opened when the test starts, the about:preferences page will load in that tab, and when I close it at the end, it closes the browser.
I added a check for gBrowser.tabs.length == 1 and added an about:blank tab if true, instead.

(In reply to Wes Kocher (:KWierso) from comment #5)
> Just a drive-by note while looking through try for unrelated reasons:
> The bug number in this patch is incorrect, and should be fixed before this
> lands.

Also updated the bug number in the patch, thanks for the notice.

Comment 7

[:Gijs (he/him)]

Comment on attachment 8513455 [details] [diff] [review]
v1.1

Review of attachment 8513455 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, but can you trypush against current fx-team tip, and retrigger, esp. on linux? Thanks!

Comment 8

[Mihaela Velimiroviciu (:mihaelav)]

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=9519c25e855f

Comment 9

[Mihaela Velimiroviciu (:mihaelav)]

There are no failures in the added tests.
Marking for checkin.

Comment 10

[(PTO June 19-July 7) Ryan VanderMeulen]

https://hg.mozilla.org/integration/fx-team/rev/8f5987a99c25

Comment 11

[(PTO June 19-July 7) Ryan VanderMeulen]

https://hg.mozilla.org/mozilla-central/rev/8f5987a99c25