Closed Bug 826311 Opened 7 years ago Closed 7 years ago

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

Categories

(Firefox :: Private Browsing, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 20

People

(Reporter: daniela.p98911, Assigned: raymondlee)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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
Blocks: pbngentest
Assignee: nobody → raymond
Attached patch v1 (obsolete) — Splinter Review
Attachment #697966 - Flags: review?(josh)
(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
Status: NEW → ASSIGNED
Comment on attachment 697966 [details] [diff] [review]
v1

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

You forgot to add the html file.
Attachment #697966 - Flags: review?(josh) → review-
Duplicate of this bug: 806728
Attached patch v2Splinter Review
Added the html
Attachment #697966 - Attachment is obsolete: true
Attachment #698006 - Flags: review?(ehsan)
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 on attachment 698006 [details] [diff] [review]
v2

Clearing the request pending an answer to comment 6.
Attachment #698006 - Flags: review?(ehsan)
(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 on attachment 698006 [details] [diff] [review]
v2

Hmm, OK, this is fine for now, I guess.
Attachment #698006 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/86c90ae23859
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.