Closed Bug 806744 Opened 7 years ago Closed 7 years ago

Port the satchel test_privbrowsing.html to the new per-tab PB APIs

Categories

(Firefox :: Private Browsing, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 20

People

(Reporter: ehsan, Assigned: marioalv)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/satchel/test/test_privbrowsing.html

In order to port this test, the file needs to be copied to the same directory (perhaps with "_perwindowpb" appended to its file name), and then instead of setting privateBrowsingEnabled, we need to open a new private browsing window and then run the test on that window.  Note that the original test should only be added to the list of test files in Makefile.in ifndef MOZ_PER_WINDOW_PRIVATE_BROWSING, and the new test file should be added to the list with the reverse condition.
Removed this test from per-window PB builds: https://hg.mozilla.org/mozilla-central/rev/6b2363d92010
Assignee: nobody → marioalv.mozilla
Status: NEW → ASSIGNED
Hi.
I changed the logic of the original test so the new logic make sense in the new per-window private browsing mode.

The steps for this test are:
First we test the transition from normal to per-window pb mode, where the form value should exist in the history.
1. Open a form in normal window mode. Fill and submit the form. 
2. Open a form in per-window private browsing mode. The value from step 1 should exist in the form history. Submit the form.
Second, we test the transition from per-window pb mode to normal mode, where the form value should no exist in the history.
3. Clear the form history so we start the test from scratch.
4. Open a form in per-window private browsing mode. Fill and submit the form. 
5. Open a form in normal window mode. The value from step 4 should not exist in the form history because we are coming from per-window private mode. Fill and submit the form. 
6. End test.

Let me know if this way of testing is OK and adapts to the new per-window private browsing mode.

Thanks.
Attachment #692993 - Flags: review?(josh)
Comment on attachment 692993 [details] [diff] [review]
Patch to migrate the test to the new per window private mode

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

r=me with the comments addressed.

::: toolkit/components/satchel/test/test_privbrowsing.html
@@ +20,5 @@
> +      // because the first load submits the form and the page reloads after
> +      // the form submission.
> +      aWindow.gBrowser.selectedBrowser.addEventListener("load", function onLoad() {
> +        aWindow.gBrowser.selectedBrowser.removeEventListener("load", onLoad, true);
> +        executeSoon(function() aCallback());

executeSoon(aCallback)

@@ +36,5 @@
> +      windowsToClose.push(aWin);
> +      // execute should only be called when need, like when you are opening
> +      // web pages on the test. If calling executeSoon() is not necesary, then
> +      // call whenNewWindowLoaded() instead of testOnWindow() on your test.
> +      executeSoon(function() aCallback(aWin));

This comment can go.

@@ +41,5 @@
> +    });
> +  };
> +
> +   // This function is called after calling finish() on the test.
> +  registerCleanupFunction(function() {

As can this one.

@@ +50,5 @@
> +
> +  // Test first when not on private mode.
> +  testOnWindow({}, function(aWin) {
> +    doTest(false, false, aWin, function() {
> +      // Then test when on private mode. The form value should be preserved.

I don't think this particular test adds any value.

@@ +60,5 @@
> +            formHistory.removeEntriesForName("field");
> +            doTest(true, false, aWin, function() {
> +              // Test when not on private mode after visiting a site on private
> +              // mode. The form history should no exist.
> +              testOnWindow({}, function(aWin) {

I don't think this test is necessary, it just verifies the result from the last one.
Attachment #692993 - Flags: review?(josh) → review+
Hi.
I made the changes you requested. 
These are now the steps executed by the test:

1. Open a form in per-window private browsing mode. Fill and submit the form. 
2. Open a form in normal window mode. The value from step 1 should not exist in the form history because we are coming from per-window private mode. Fill and submit the form. 
3. End test.

Please let me know if everything's OK with the changes.

Thanks.
Attachment #692993 - Attachment is obsolete: true
Attachment #694025 - Flags: review?(josh)
Attachment #694025 - Flags: review?(josh)
Josh, do we need a try run for this?
Yes, it would be good. This may also break mobile.
Try is good!
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a3f4fd5bd563
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 20
You need to log in before you can comment on or make changes to this bug.