493671 - Mozmill ignores prefs, does not reflect manual behaviour

Status: RESOLVED WORKSFORME

(Testing Graveyard :: Mozmill, defect, P1)

Description

[u279076]

I've been trying to run a test case to check that the popup blocker is working.  When I run the test, the popups appear with no warning bar even though the pref is set.  In the UI "Block Pop-ups" remains checked and in about:config the privacy.popups.policy pref is set to 1.

The test case attempts to load a local html file which tries to create a popup.  If I open the test file manually (i.e. Not with Mozmill), the popup is blocked and I see the yellow warning bar.  However, loading the html file in my testcase using controller.open(url), pop ups appear.

I'll attach the test case in question.

Comment 1

[u279076]

Attachment: TEST CASE

Here is the JS test case.  HTML test site to follow.

Comment 2

[u279076]

Attachment: TEST SITE

This is the HTML test site.  It needs to be in a "files" subfolder of the JS test case.

Example:
5818r5.js
files
   index.html

Comment 3

[u279076]

After further investigation, it appears to be something involving the waitForElement method.

var testWarningAppears = function() {  
  // Ensure that the "Block Pop-ups" pref is checked
  ensurePrefChecked();
  // Open the Pop-up test site
  controller.open(url+'index.html');
  // Check that the warning bar exists
  var elem = new elementslib.Lookup(controller.window.document, '/id("main-window")/id("browser")/id("appcontent")/id("content")/anon({"anonid":"tabbox"})/anon({"anonid":"panelcontainer"})/[0]/{"value":"popup-blocked","priority":"5","type":"warning"}/anon({"class":"notification-inner outset","flex":"1","xbl:inherits":"type","type":"warning"})/anon({"anonid":"details"})/anon({"anonid":"messageText"})');
  controller.waitForElement(elem, 10000, 10);
  controller.assertNode(elem);

}

Commenting out the last two lines of this method causes the test to behave as expected in the browser (pop-ups blocked, warning bar displays)

Comment 4

[u279076]

Marking critical as per offline discussion with ctalbert.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Works fine for me. The pref is correctly switched on and the page is loaded with the yellow notification bar. I'm not able to see the above mentioned problems. Anthony, how many tabs you have open while running the test and in which tab you run it?

Comment 6

[u279076]

I've reproduced this from any number of tabs (0-5).  It does not happen every time, but it does happen a lot (as ctalbert has witnessed and reproduced on his own computer).  Using the full test file, not just the stuff in comment 3, I can reproduce this.  When testing, please try against three different start pages:

1. about:blank
2. page you see when the test completes
3. home page

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

As said, works fine with Shiretoko and Minefield + Mozmill trunk. I cannot see it failing at any time after more then 10 test-runs. Seems like Clint has to step in here.

Comment 8

[cmtalbert]

It's a race condition.  Though I'm not exactly sure what we're racing.  It looks like we push a openAllowed popupControlState onto gPopupControlState in nsDocShell::LoadURI. 

However, if we get down to test the gPopupControlState in the nsGlobalWindow::CheckForAbusePoint (called from nsGlobalWindow::OpenInternal) quickly then it gets a value of openAbused from gPopupControlState and prevents the popup from opening.

If you wait, then gPopupControlState gets the openAllowed value and the popup is loaded.  

I'm not at all clear what is going on here.  These are the breakpoints I'm using:
13  breakpoint     keep y   0x12c4c938 in nsGlobalWindow::PopPopupControlState(PopupControlState) const at /Users/clint/code/mozcentral/src/dom/base/nsGlobalWindow.cpp:1409
	breakpoint already hit 12 times
14  breakpoint     keep y   0x12c4c91e in nsGlobalWindow::PushPopupControlState(PopupControlState, int) const at /Users/clint/code/mozcentral/src/dom/base/nsGlobalWindow.cpp:1403
15  breakpoint     keep y   0x123d8112 in nsDocShell::LoadURI(unsigned short const*, unsigned int, nsIURI*, nsIInputStream*, nsIInputStream*) at /Users/clint/code/mozcentral/src/docshell/base/nsDocShell.cpp:3393
	breakpoint already hit 15 times
10  breakpoint     keep y   0x12c6798e in nsGlobalWindow::OpenInternal(nsAString_internal const&, nsAString_internal const&, nsAString_internal const&, int, int, int, int, nsIArray*, nsISupports*, nsIPrincipal*, JSContext*, nsIDOMWindow**) at /Users/clint/code/mozcentral/src/dom/base/nsGlobalWindow.cpp:7242

My line numbers are off because the build is a few days old.

I'll rerun with the latest, but this is what I've found.

Essentially if you delay your debugging long enough to get a slow script dialog (or anything that hits the event loop) then you will see the popup fired.  If you are quick with your debugging then you will see the popup get blocked.  You can also simulate this by uncommenting the controller.waitForPageLoad line just after the controller.open call.

Comment 9

[cmtalbert]

Attachment: A simple mozmill test that demonstrates the issue

Comment 10

[cmtalbert]

Mr. Bkap, do you have any ideas as to what is going on here?

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #9)
> Created an attachment (id=378746) [details]
> A simple mozmill test that demonstrates the issue

Even with this test I'm not able to reproduce the problem.

Comment 12

[cmtalbert]

(In reply to comment #11)
> (In reply to comment #9)
> > Created an attachment (id=378746) [details] [details]
> > A simple mozmill test that demonstrates the issue
> 
> Even with this test I'm not able to reproduce the problem.
Henrik, what are you testing with?  I can repro it at will using this test case and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090520 Minefield/3.6a1pre

Steps:
1. Load test case as is --> popup blocked (good)
2. Load test case with the waitForPageLoad line uncommented --> popup shown (bad).

Step 2 is the bad one and the problem.  You can't reproduce this whimboo?  If you can't that supports my hypothesis that this is a race condition.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I run it with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090520 Minefield/3.6a1pre ID:20090520033508 and Mozmill r464 (latest trunk).

Jupp, step 1 and 2 are fine here. Popups are getting blocked in both cases.

Comment 14

[Mikeal Rogers [:mikeal] (mrogers@)]

Is this still a valid bug?

Comment 15

[u279076]

(In reply to comment #14)
> Is this still a valid bug?

I'll test this tomorrow with the latest Mozmill and the test that was failing.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Anthony, any update from your side? Does it still happen for you?

Comment 17

[u279076]

Sorry...I didn't have a chance to check this out today.  I'm heading home right now but I'll check tonight.

Comment 18

[u279076]

Just checked this again using Clint's minimized test case.  Run through Mozmill, it just reloads the current tab.  However, if I copy and paste the URL from the test (http://people.mozilla.org/~ctalbert/mozmilltest.html), a pop-up warning bar appears.  

Replacing the URL in the test case with http://www.popuptest.com/popuptest1.html, pop-ups are not blocked even though the pref is set, when run through Mozmill.  Manually, popups are blocked and the warning bar appears.

Comment 19

[Aakash Desai [:aakashd]]

This is just a nit, but the [mozmill] tag is used only for testscript bugs not for bugs on mozmill. The summary has had this removed :)

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Anthony, can you please try again? I wonder if this was somehow related to our waitForPageLoad problem which has been fixed a couple of hours ago.

Comment 21

[u279076]

The following code works.  I ran this 20 times in succession.  I get no failures and the prefs dialog seems to behave as it should.  Is this what you wanted tested?  I'm a bit confused (as usual).


var setupModule = function(module) {
  module.controller = mozmill.getBrowserController();
}

function delayedAssertNode(controller, aNode, aTimeout) {
  controller.waitForElement(aNode, aTimeout);
  controller.assertNode(aNode);
}

var testFoo = function(){
  var prefController = mozmill.getPreferencesController();
  var paneMain = '/id("BrowserPreferences")/anon({"orient":"vertical"})/anon({"anonid":"selector"})/{"pane":"paneMain"}';
  prefController.click(new elementslib.Lookup(prefController.window.document, paneMain));

  // Check if the Main pane is active
  var node = new elementslib.ID(prefController.window.document, 'browserStartupPage');
  delayedAssertNode(prefController, node, 3000);

  // Close the preferences dialog
  prefController.keypress(null, 'VK_ESCAPE', {});
}

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Anthony, see your comment 18. If everything is fine we can go on and finalize the test.

Comment 23

[u279076]

Clint's minimized test case appears to work now.  Popup is blocked and warning bar appears.  I tested it 20 times in succession without fail.

Comment 24

[Tony Chung [:tchung]]

great, so is this bug fixed?  if so, can we mark it appropriately, and get an update on the mozmill test?  Thanks.

Comment 25

[Henrik Skupin [:whimboo][⌚️UTC+1]]

As Anthony told me on IRC this should have been WFM now. I'll mark it so and update the spreadsheet.