806683 - Port browser_privatebrowsing_crh.js to the new per-window PB APIs

Status: RESOLVED FIXED

(Firefox :: Private Browsing, defect)

Description

[(no longer active)]

http://mxr.mozilla.org/mozilla-central/source/browser/components/privatebrowsing/test/browser/global/browser_privatebrowsing_crh.js

In order to port this test, the file needs to be copied to the perwindow/ directory, and then instead of setting privateBrowsingEnabled sequentially, the test needs to open a new private browsing window and then run the test on that window.  This involves rewriting the test structure to make it asynchronous.

Comment 1

[Andres Hernandez [:andreshm]]

Attachment: Patch v1

Comment 2

[Josh Matthews [:jdm]]

Comment on attachment 679169 [details] [diff] [review]
Patch v1

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

r=me with these nits addressed.

::: browser/components/privatebrowsing/test/browser/perwindow/browser_privatebrowsing_crh.js
@@ +8,5 @@
> +function test() {
> +  waitForExplicitFinish();
> +
> +  function checkDisableOption(aPrivateMode, aWindow, aCallback) {
> +    executeSoon(function() {

Does the test pass without this?

@@ +10,5 @@
> +
> +  function checkDisableOption(aPrivateMode, aWindow, aCallback) {
> +    executeSoon(function() {
> +      let crhCommand = aWindow.document.getElementById("Tools:Sanitize");
> +      ok(crhCommand, "The chr command should exist");

Be explicit here: Clear Recent History, not chr.

@@ +13,5 @@
> +      let crhCommand = aWindow.document.getElementById("Tools:Sanitize");
> +      ok(crhCommand, "The chr command should exist");
> +
> +      is(PrivateBrowsingUtils.isWindowPrivate(aWindow), aPrivateMode,
> +        "PrivateBrowsingUtils should accept the correct per-window private browsing status");

s/accept/report/

Comment 3

[Andres Hernandez [:andreshm]]

Attachment: Patch v2

> browser/components/privatebrowsing/test/browser/perwindow/
> browser_privatebrowsing_crh.js
> @@ +8,5 @@
> > +function test() {
> > +  waitForExplicitFinish();
> > +
> > +  function checkDisableOption(aPrivateMode, aWindow, aCallback) {
> > +    executeSoon(function() {
> 
> Does the test pass without this?

No, the test fails without that. I suppose that the attribute hasn't been set.

Comment 4

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7de203873898

(Nit: please use hg cp in the future.  Thanks!)

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7de203873898