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

RESOLVED FIXED in Firefox 20

Status

()

Firefox
Private Browsing
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Ehsan, Assigned: raymondlee)

Tracking

(Blocks: 1 bug)

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

Firefox Tracking Flags

(firefox20+ fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

6 years ago
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/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

5 years ago
Removed this test from per-window PB builds: https://hg.mozilla.org/mozilla-central/rev/cf47dba80a4e
(Assignee)

Updated

5 years ago
Assignee: nobody → raymond
(Assignee)

Comment 2

5 years ago
Created attachment 692916 [details] [diff] [review]
v1
Attachment #692916 - Flags: review?(ehsan)
(Assignee)

Updated

5 years ago
Status: NEW → ASSIGNED
(Reporter)

Updated

5 years ago
Attachment #692916 - Flags: review?(ehsan) → review?(josh)
Comment on attachment 692916 [details] [diff] [review]
v1

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

::: toolkit/components/passwordmgr/test/test_privbrowsing.html
@@ +192,5 @@
> +  var win = mainWindow.OpenBrowserWindow({private: aIsPrivate});
> +  win.addEventListener("load", function onLoad() {
> +    win.removeEventListener("load", onLoad, false);
> +    win.addEventListener("DOMContentLoaded", function onInnerLoad() {
> +      // I am at my wits' end figuring out how to stop this load from occurring. I give up.

Just remove this comment to stop it propagating to other tests.
Attachment #692916 - Flags: review?(josh) → review+
(Assignee)

Comment 4

5 years ago
Created attachment 693454 [details] [diff] [review]
v2

(In reply to Josh Matthews [:jdm] from comment #3)
> > +      // I am at my wits' end figuring out how to stop this load from occurring. I give up.
> 
> Just remove this comment to stop it propagating to other tests.

Removed that comment
Attachment #692916 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
And backed out for Android test failures. Tryserver kthxbai.
https://hg.mozilla.org/integration/mozilla-inbound/rev/b3d7e749388b

https://tbpl.mozilla.org/php/getParsedLog.php?id=18070140&tree=Mozilla-Inbound

76777 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/components/passwordmgr/test/test_privbrowsing_perwindowpb.html | uncaught exception - TypeError: mainWindow.OpenBrowserWindow is not a function at http://mochi.test:8888/tests/toolkit/components/passwordmgr/test/test_privbrowsing_perwindowpb.html:192
12-18 15:11:13.792 I/GeckoDump( 1834): 76777 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/components/passwordmgr/test/test_privbrowsing_perwindowpb.html | uncaught exception - TypeError: mainWindow.OpenBrowserWindow is not a function at http://mochi.test:8888/tests/toolkit/components/passwordmgr/test/test_privbrowsing_perwindowpb.html:192
(Assignee)

Comment 7

5 years ago
(In reply to Ryan VanderMeulen from comment #6)
> And backed out for Android test failures. Tryserver kthxbai.
> https://hg.mozilla.org/integration/mozilla-inbound/rev/b3d7e749388b
> 
> https://tbpl.mozilla.org/php/getParsedLog.php?id=18070140&tree=Mozilla-
> Inbound
> 
> 76777 ERROR TEST-UNEXPECTED-FAIL |
> /tests/toolkit/components/passwordmgr/test/test_privbrowsing_perwindowpb.
> html | uncaught exception - TypeError: mainWindow.OpenBrowserWindow is not a
> function at
> http://mochi.test:8888/tests/toolkit/components/passwordmgr/test/
> test_privbrowsing_perwindowpb.html:192
> 12-18 15:11:13.792 I/GeckoDump( 1834): 76777 ERROR TEST-UNEXPECTED-FAIL |
> /tests/toolkit/components/passwordmgr/test/test_privbrowsing_perwindowpb.
> html | uncaught exception - TypeError: mainWindow.OpenBrowserWindow is not a
> function at
> http://mochi.test:8888/tests/toolkit/components/passwordmgr/test/
> test_privbrowsing_perwindowpb.html:192

@ehsan / josh: The issues only happen in Android.  Any suggestions how to fix this?
(Reporter)

Comment 8

5 years ago
Firefox for Android's browser.js does not implement OpenBrowserWindow.  I'm not sure what happens when you try to open a new window on mobile.  Brian, how do you open a new private tab on mobile from a mochitest?
(In reply to :Ehsan Akhgari from comment #8)
> Brian, how do you open a new private tab on mobile from a mochitest?

I think you can do this:
mainWindow.BrowserApp.addTab(url, { isPrivate: true });
(Assignee)

Comment 10

5 years ago
Created attachment 698579 [details] [diff] [review]
v3

I have updated the patch, however, the getPopupNotification(aWindow) in notification_common.js (http://mxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/test/notification_common.js#6) doesn't return undefined for Andriod.  I think chromeWin.PopupNotifications doesn't exist in Andriod.

Also, all tests which invoke getPopupNotifications are excluded for Android e.g. test_notifications.html, test_prompt.html in http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/android.json.  

Furthermore, test_privbrowsing.html is not in that list, I think it's because it has a condition to check whether nsIPrivateBrowsingService exists or not before executing the test, and Andriod doesn't have that API.

Added the test file to excluded list, and everything goes ok.
https://tbpl.mozilla.org/?tree=Try&rev=7f079b94f96c

Without adding the file to excluded list and it fails with no popupNotifications.
https://tbpl.mozilla.org/?tree=Try&rev=929e5cdc04e4

@brian: could you confirm that please?
Attachment #693454 - Attachment is obsolete: true
Attachment #698579 - Flags: review?(ehsan)
Flags: needinfo?(bnicholson)
(Reporter)

Updated

5 years ago
Attachment #698579 - Flags: review?(ehsan) → review+
(In reply to Raymond Lee [:raymondlee] from comment #10)
> Created attachment 698579 [details] [diff] [review]
> v3
> 
> @brian: could you confirm that please?

Yes, that makes sense; I think disabling this was the right thing to do. CC'ing gbrown in case he has any objections.
Flags: needinfo?(bnicholson)
Seems reasonable to me.
(Assignee)

Comment 15

5 years ago
Created attachment 699635 [details] [diff] [review]
v4

I have made some changes to fix frequent intermittent failures.  Pushed to try twice and look good!

https://tbpl.mozilla.org/?tree=Try&rev=015caaaa6e85
https://tbpl.mozilla.org/?tree=Try&rev=c8097176c5b4
Attachment #698579 - Attachment is obsolete: true
Attachment #699635 - Flags: review?(ehsan)
(Reporter)

Updated

5 years ago
Attachment #699635 - Flags: review?(ehsan) → review+
(Reporter)

Comment 17

5 years ago
Alex, do we need Aurora approval for test-only changes?
https://hg.mozilla.org/mozilla-central/rev/bdddc01216a4
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
(Reporter)

Comment 19

5 years ago
Raymond, can you please attach a version of this patch which applies cleanly on top of Aurora?  Thanks!
(Assignee)

Comment 20

5 years ago
Comment on attachment 699635 [details] [diff] [review]
v4

Ehsan, this last patch in this bug can apply cleanly to the tip of aurora.  

changeset:   123620:1fff5df1ac6a
tag:         tip
user:        Dão Gottwald <dao@mozilla.com>
date:        Mon Jan 14 10:21:10 2013 +0100
summary:     Bug 824825 - Add toolbar button and popup notification icons for tabs with camera / microphone access. r=dolske a=lsblakk

Could you try please?
(Reporter)

Comment 21

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/c06d65b1674a

(I think I posted this on the wrong bug.  Oh well...)
tracking-firefox20: --- → +
(Reporter)

Comment 22

5 years ago
Hmm, this is failing on Aurora:

https://tbpl.mozilla.org/?tree=Mozilla-Aurora&rev=12f52471747d

So I backed it out: https://hg.mozilla.org/releases/mozilla-aurora/rev/1292047d5d18

Raymond, can you please investigate?  Thanks!
(Assignee)

Comment 23

5 years ago
(In reply to :Ehsan Akhgari from comment #22)
> Hmm, this is failing on Aurora:
> 
> https://tbpl.mozilla.org/?tree=Mozilla-Aurora&rev=12f52471747d
> 
> So I backed it out:
> https://hg.mozilla.org/releases/mozilla-aurora/rev/1292047d5d18
> 
> Raymond, can you please investigate?  Thanks!

Ehsan, I think the wrong version of patch was applied.  It was using v3, could you try v4 again please?
(Reporter)

Comment 25

5 years ago
I'll do that when Aurora reopens.  Thanks!
Whiteboard: [land-on-aurora]
(Reporter)

Updated

5 years ago
Keywords: checkin-needed
(Reporter)

Comment 26

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/5020af3460e0
status-firefox20: --- → fixed
Keywords: checkin-needed
Whiteboard: [land-on-aurora]
You need to log in before you can comment on or make changes to this bug.