The default bug view has changed. See this FAQ.

Copy and port current preference tests to run against the in-content preferences

RESOLVED FIXED in Firefox 15

Status

()

Firefox
Preferences
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Zuhao(Joe) Chen, Assigned: Zuhao(Joe) Chen)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 15
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 6 obsolete attachments)

(Assignee)

Description

5 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_2) AppleWebKit/535.11 (KHTML, like Gecko) Chrome/17.0.963.56 Safari/535.11

Steps to reproduce:

Testing for in-content preferences


Actual results:

Testing for in-content preferences


Expected results:

Testing for in-content preferences
(Assignee)

Updated

5 years ago
Summary: in-content preferences testing → Testing for In-content Preferences
(Assignee)

Updated

5 years ago
Blocks: 718011
Status: UNCONFIRMED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 651803
This bug is not a duplicate. It is part of the project to move preferences to in-content UI.

There are a number of tests located at /browser/components/preferences/tests that will need to be copied to /browser/components/preferences/in-content/tests
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: DUPLICATE → ---
Status: REOPENED → ASSIGNED
Component: Untriaged → Preferences
OS: Mac OS X → All
QA Contact: untriaged → preferences
Hardware: x86 → All
Version: unspecified → Trunk
(Assignee)

Comment 3

5 years ago
Created attachment 605198 [details] [diff] [review]
Tests for in-content preferences
Attachment #605198 - Flags: review?(jwein)
Attachment #605198 - Flags: review?(bmcbride)
Comment on attachment 605198 [details] [diff] [review]
Tests for in-content preferences

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

From a cursory glance, this looks to be what in the right direction, but they're still testing against the old user interface.

::: browser/components/preferences/in-content/tests/Makefile.in
@@ +1,2 @@
> +# ***** BEGIN LICENSE BLOCK *****
> +# Version: MPL 1.1/GPL 2.0/LGPL 2.1

MPL 2.0 here.

::: browser/components/preferences/in-content/tests/privacypane_tests.js
@@ +56,5 @@
> +    }
> +  };
> +  Services.ww.registerNotification(observer);
> +
> +  let dialog = openDialog("chrome://browser/content/preferences/preferences.xul", "Preferences",

These tests are opening the old preferences, so they will need to be updated to use the in-content prefs.

These tests should fail if you don't have the patch for the privacy preferences applied :)
Attachment #605198 - Flags: review?(jwein) → feedback+
(Assignee)

Comment 5

5 years ago
Tried to run the test with the following command:
TEST_PATH=browser/components/preferences/in-content/tests/ make -C obj-ff-dbg/ mochitest-plain

but it's giving me the error test not found. Any advise?

(In reply to Jared Wein [:jaws] from comment #4)
> Comment on attachment 605198 [details] [diff] [review]
> Tests for in-content preferences
> 
> Review of attachment 605198 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> From a cursory glance, this looks to be what in the right direction, but
> they're still testing against the old user interface.
> 
> ::: browser/components/preferences/in-content/tests/Makefile.in
> @@ +1,2 @@
> > +# ***** BEGIN LICENSE BLOCK *****
> > +# Version: MPL 1.1/GPL 2.0/LGPL 2.1
> 
> MPL 2.0 here.
> 
> ::: browser/components/preferences/in-content/tests/privacypane_tests.js
> @@ +56,5 @@
> > +    }
> > +  };
> > +  Services.ww.registerNotification(observer);
> > +
> > +  let dialog = openDialog("chrome://browser/content/preferences/preferences.xul", "Preferences",
> 
> These tests are opening the old preferences, so they will need to be updated
> to use the in-content prefs.
> 
> These tests should fail if you don't have the patch for the privacy
> preferences applied :)
Assignee: nobody → joejoevictor
Joe: Did you sort out the issues with running the tests? Do you have an updated patch?

(Sorry, I'm a bit behind on the status of things after being off sick.)
Attachment #605198 - Flags: review?(bmcbride)
Summary: Testing for In-content Preferences → Copy and port current preference tests to run against the in-content preferences
Joe: Have you made any progress on this?

Comment 8

5 years ago
Created attachment 611680 [details] [diff] [review]
individual tests

attempt at getting all the old tests to pass, most seem to be passing but having a few give one or two tests failing

Comment 9

5 years ago
Created attachment 612562 [details] [diff] [review]
in-content tests
Attachment #611680 - Attachment is obsolete: true
Comment on attachment 612562 [details] [diff] [review]
in-content tests

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

Jon, can you push all your patches and this patch to tryserver to see if these tests pass on all platforms?

::: browser/components/preferences/in-content/preferences.js
@@ +11,5 @@
>  Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
>  
>  function init_all() {
> +  var initFinished = document.createEvent("Event");
> +  initFinished.initEvent("Initialized", true, true);

Move these two lines to the end of init_all, and use |let| instead of |var|.

::: browser/components/preferences/in-content/privacy.js
@@ +163,4 @@
>        this.dependentControls
>            .forEach(function (aElement)
>                     document.getElementById(aElement).disabled = disabled);
> +                   

Extraneous whitespace got added here.

::: browser/components/preferences/in-content/tests/browser_bug705422.js
@@ +36,5 @@
> +        cookieSvc.setCookieString(cookieUri, null, name+"="+value+";", null);
> +    }
> +
> +    // open cookie manager
> +    var cmd = window.openDialog("chrome://browser/content/preferences/in-content/cookies.xul",

Does cookies.xul need to be copied to the in-content folder? Or can we just use the current cookies.xul?

::: browser/components/preferences/in-content/tests/browser_bug731866.js
@@ +16,5 @@
> +  Services.obs.addObserver(observer, "browser-preferences-initialized", false);
> +
> +  // open about:preferences
> +  gBrowser.removeCurrentTab();
> +  gBrowser.selectedTab = gBrowser.addTab("chrome://browser/components/preferences/in-content/preferences.xul");

This should use ABOUT_PREFERENCES_SPEC instead of the full chrome URL.

::: browser/components/preferences/in-content/tests/privacypane_tests.js
@@ +11,5 @@
> +    browser.addEventListener("Initialized", function(aEvent) {
> +      dump("\nInitialized\n");
> +      browser.removeEventListener("Initialized", arguments.callee, true);
> +      setTimeout(function() {
> +  //      is(Services.prefs.getBoolPref("browser.preferences.inContent"), true, "In-content prefs are enabled");

These comments should be removed,

@@ +13,5 @@
> +      browser.removeEventListener("Initialized", arguments.callee, true);
> +      setTimeout(function() {
> +  //      is(Services.prefs.getBoolPref("browser.preferences.inContent"), true, "In-content prefs are enabled");
> +        is(browser.contentWindow.location.href, "about:preferences", "Checking if the preferences tab was opened");
> +        dump("\n here1 \n");

as well as these dump statements,

@@ +36,5 @@
> +//    testFunc(win.document.defaultView);
> +//    gBrowser.removeCurrentTab();
> +//    dump("\n here2 \n");
> +//    testRunner.runNext();
> +//  }, false);

and these comments too :)

@@ +485,5 @@
> +}
> +
> +let testRunner;
> +function run_test_subset(subset) {
> +  let instantApplyOrig = Services.prefs.getBoolPref("browser.preferences.instantApply");

We don't need to store the original pref value, see below.

@@ +496,5 @@
> +    counter: 0,
> +    runNext: function() {
> +      if (this.counter == this.tests.length) {
> +        // cleanup
> +        Services.prefs.setBoolPref("browser.preferences.instantApply", instantApplyOrig);

Services.prefs.clearUserPref("browser.preferences.instantApply");
and use registerCleanupFunction at the beginning of test() to reset this pref.
Attachment #612562 - Flags: feedback+

Comment 11

5 years ago
Created attachment 613790 [details] [diff] [review]
copied over tests + browser_bug731866.js patch

permissions and cookies tests were removed and browser_bug731866.js for testing pane switching was added. All pass as well : )
Attachment #612562 - Attachment is obsolete: true
Attachment #613790 - Flags: feedback?(jwein)
Attachment #613790 - Flags: feedback?(bmcbride)
Comment on attachment 613790 [details] [diff] [review]
copied over tests + browser_bug731866.js patch

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

::: browser/components/preferences/in-content/preferences.js
@@ +13,5 @@
>  Components.utils.import("resource://services-sync/service.js");
>  
>  function init_all() {
> +  var initFinished = document.createEvent("Event");
> +  initFinished.initEvent("Initialized", true, true);

Please move these two lines to be right before the event is dispatched.

::: browser/components/preferences/in-content/tests/privacypane_tests.js
@@ +7,5 @@
> +    gBrowser.tabContainer.removeEventListener("TabOpen", arguments.callee, true);
> +    let browser = aEvent.originalTarget.linkedBrowser;
> +    browser.addEventListener("Initialized", function(aEvent) {
> +      browser.removeEventListener("Initialized", arguments.callee, true);
> +//      is(Services.prefs.getBoolPref("browser.preferences.inContent"), true, "In-content prefs are enabled");

Hmm, was this meant to be commented out? Is this check failing for some reason?
Attachment #613790 - Flags: feedback?(jwein) → feedback+

Comment 13

5 years ago
Created attachment 616401 [details] [diff] [review]
patch for testing

Comment 14

5 years ago
Created attachment 617001 [details] [diff] [review]
patch for tests
Attachment #613790 - Attachment is obsolete: true
Attachment #616401 - Attachment is obsolete: true
Attachment #617001 - Flags: feedback?(bmcbride)
Attachment #613790 - Flags: feedback?(bmcbride)
Attachment #605198 - Attachment is obsolete: true

Comment 15

5 years ago
Created attachment 617048 [details] [diff] [review]
patch for tests
Attachment #617001 - Attachment is obsolete: true
Attachment #617048 - Flags: feedback?(bmcbride)
Attachment #617001 - Flags: feedback?(bmcbride)
Attachment #617048 - Flags: review+
Attachment #617048 - Flags: feedback?(bmcbride)
\o/ Pushed to mozilla-inbound. These patches should be merged to mozilla-central within the next day or two. Thanks!

https://hg.mozilla.org/integration/mozilla-inbound/rev/8b195889f55c
Flags: in-testsuite+
Target Milestone: --- → Firefox 15
https://hg.mozilla.org/mozilla-central/rev/8b195889f55c
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Depends on: 820815
You need to log in before you can comment on or make changes to this bug.