826311 - Extend the browser_privatebrowsing_cookieacceptdialog.js to check if the cookies modal dialog appears

Status: RESOLVED FIXED

(Firefox :: Private Browsing, defect)

Description

[Daniela Petrovici]

The existing browser_privatebrowsing_cookieacceptdialog.js test needs to be updated to contain the following steps:
 1) Have the Accept cookies from sites option set in Firefox for ask me every time
 2) Open New Private Window
 3) Verify that the Confirm setting cookie dialog is not displayed
 4) Open New Window
 5) Verify that the Confirm setting cookie dialog is displayed
At the moment the test is not verifying that the cookies modal dialog appears

Comment 1

[Raymond Lee [:raymondlee]]

Attachment: v1

Comment 2

[Raymond Lee [:raymondlee]]

(In reply to Raymond Lee [:raymondlee] from comment #1)
> Created attachment 697966 [details] [diff] [review]
> v1

I think the patch might cover Bug 806728 comment 2

Comment 3

[(no longer active)]

Comment on attachment 697966 [details] [diff] [review]
v1

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

You forgot to add the html file.

Comment 5

[Raymond Lee [:raymondlee]]

Attachment: v2

Added the html

Comment 6

[(no longer active)]

Comment on attachment 698006 [details] [diff] [review]
v2

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

::: browser/components/privatebrowsing/test/browser/perwindow/browser_privatebrowsing_cookieacceptdialog.js
@@ +79,5 @@
> +    function onLoad() {
> +      selectedBrowser.removeEventListener("load", onLoad, true);
> +      Services.ww.unregisterNotification(observer);
> +
> +      ok(aIsPrivateWindow, "Confirm setting dialog is not displayed for private window");

Hmm, will this not fail for non-private windows?  The onLoad handler should be called for all windows since you just load a URL inside the browser, or am I missing something?

Comment 7

[(no longer active)]

Comment on attachment 698006 [details] [diff] [review]
v2

Clearing the request pending an answer to comment 6.

Comment 8

[Raymond Lee [:raymondlee]]

(In reply to :Ehsan Akhgari from comment #6)
> Comment on attachment 698006 [details] [diff] [review]
> v2
> 
> Review of attachment 698006 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> :::
> browser/components/privatebrowsing/test/browser/perwindow/
> browser_privatebrowsing_cookieacceptdialog.js
> @@ +79,5 @@
> > +    function onLoad() {
> > +      selectedBrowser.removeEventListener("load", onLoad, true);
> > +      Services.ww.unregisterNotification(observer);
> > +
> > +      ok(aIsPrivateWindow, "Confirm setting dialog is not displayed for private window");
> 
> Hmm, will this not fail for non-private windows?  The onLoad handler should
> be called for all windows since you just load a URL inside the browser, or
> am I missing something?

Actually no, because "domwindowopened" topic would be notified for no-private window and the observer() would be invoked before the load() and the load event listener would be cancelled inside observer.  May be we can improve the test by using a condition to only add load listener for private window and observer for non-private window?

Comment 9

[(no longer active)]

Comment on attachment 698006 [details] [diff] [review]
v2

Hmm, OK, this is fine for now, I guess.

Comment 10

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/86c90ae23859

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/86c90ae23859