All users were logged out of Bugzilla on October 13th, 2018

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

RESOLVED FIXED in Firefox 20

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Ehsan, Assigned: marioalv)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 20
x86
Mac OS X
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
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.
(Reporter)

Comment 1

6 years ago
Removed this test from per-window PB builds: https://hg.mozilla.org/mozilla-central/rev/6b2363d92010
Assignee: nobody → marioalv.mozilla
(Assignee)

Updated

6 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 2

6 years ago
Created attachment 692993 [details] [diff] [review]
Patch to migrate the test to the new per window private mode

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 3

6 years ago
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+
(Assignee)

Comment 4

6 years ago
Created attachment 694025 [details] [diff] [review]
Patch to migrate the test to the new per window private mode

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?

Comment 6

6 years ago
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
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 20
You need to log in before you can comment on or make changes to this bug.