Closed Bug 806683 Opened 7 years ago Closed 7 years ago

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

Categories

(Firefox :: Private Browsing, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 19

People

(Reporter: ehsan, Assigned: andreshm)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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.
Blocks: pbngentest
Assignee: nobody → andres
Attached patch Patch v1 (obsolete) — Splinter Review
Attachment #679169 - Flags: review?(josh)
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/
Attachment #679169 - Flags: review?(josh) → review+
Attached patch Patch v2Splinter Review
> 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.
Attachment #679169 - Attachment is obsolete: true
Attachment #679323 - Flags: review?(josh)
Attachment #679323 - Flags: review?(josh) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/7de203873898

(Nit: please use hg cp in the future.  Thanks!)
https://hg.mozilla.org/mozilla-central/rev/7de203873898
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
You need to log in before you can comment on or make changes to this bug.