666636 - enable specialpowers for all mochitest types

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

currently specialpowers is only for mochitest-plain, and in order to convert the shared libraries the tests require to using SpecialPowers, we need to have SpecialPowers available to chrome/a11y/browser-chrome.

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: add specialpowers to chrome/a11y (WIP)

marking this patch as WIP  since it doesn't have the browser-chrome bits.  The browser-chrome fix is just one more line to remove and I have it running on try server after watching it work on my local linux box.  Will upload a final patch once I see green on try server.

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: add specialpowers to chrome/a11y/browser-chrome (1.0)

updated with browser-chrome port and tested on try server.

Comment 3

[(not currently active) Ted Mielczarek]

Comment on attachment 541981 [details] [diff] [review]
add specialpowers to chrome/a11y/browser-chrome (1.0)

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

Patch looks good. I just have one request, but it's r=me either way.

::: testing/mochitest/redirect.js
@@ +39,5 @@
>   * ***** END LICENSE BLOCK ***** */
>  
>  function redirect(aURL)
>  {
> +  SpecialPowers.loadURI(window, aURL + location.search,

We really need to fix this, this is awful. (Loading the HTTP page then redirecting to chrome?) Can we just fix this in the harness instead of adding a new API to make it work? Seems like we ought to be able to just load the chrome:// URL directly.

::: testing/mochitest/specialpowers/content/specialpowers.js
@@ +353,4 @@
>        return;
>      }
>  
> +    if (uri.scheme == "http" || uri.scheme == "https" || uri.scheme == "chrome") {

Why the double-check here? We've already got the scheme blacklist above.

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: updated patch to work on try (2.0)

for some reason I must have not ran this on try server (or something else changed).  I had to leave the check in SimpleTest.js to verify SpecialPowers != undefined because I ran into problems with:
mochitest-plain(2):
dom/tests/mochitest/ajax/offline/test_bug474696.html
dom/tests/mochitest/ajax/offline/test_bug544462.html (side effect of first failure?)
dom/tests/mochitest/ajax/offline/test_bypass.html (side effect?)
dom/tests/mochitest/ajax/offline/test_changingManifest.html (side effect?)
aborted test after 4 timeouts

mochitest-chrome:
dom/tests/mochitest/chrome/test_focus.xul
layout/base/test/chrome/test_default_background.xul
toolkit/content/tests/chrome/test_browser_drop.xul
toolkit/content/tests/chrome/test_keys.xul


I did remove the check for type of uri scheme, but I was not able to easily remove the redirect.html|js code which uses loadURI.

Comment 5

[Josh Matthews [:jdm]]

Enabling this for jsreftests would fix bug 669949 as well. Is that something worth considering?

Comment 6

[(not currently active) Ted Mielczarek]

jsreftests is a whole different harness. Please file a new bug.

Comment 7

[(not currently active) Ted Mielczarek]

Comment on attachment 544041 [details] [diff] [review]
updated patch to work on try (2.0)

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

I filed a separate bug on the offline tests, bug 645695, since they have a file that makes heavy use of XPCOM and I didn't see an easy way to fix it. I think it would be good to figure out why we're hitting this issue, rather than wallpaper over it, but it's hard to worry about those offline tests when they're going to need a lot of work anyway.

::: testing/mochitest/specialpowers/content/specialpowers.js
@@ +349,5 @@
>      var window = aEvent.target.defaultView;
>  
>      // Need to make sure we are called on what we care about -
>      // content windows. DOMWindowCreated is called on *all* HTMLDocuments,
>      // some of which belong to chrome windows or other special content.

Please update this comment since it's no longer correct...

@@ +352,5 @@
>      // content windows. DOMWindowCreated is called on *all* HTMLDocuments,
>      // some of which belong to chrome windows or other special content.
>      //
>      var uri = window.document.documentURIObject;
> +    dump("TEST-INFO | specialpowers.js | 31415926535 | " + uri.spec + "\n");

Hah! As cute as this is, this is going to add a bunch of log spew and you should probably remove it.

Comment 8

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

http://hg.mozilla.org/mozilla-central/rev/5dfcc562b0a9

Comment 9

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 544041 [details] [diff] [review]
updated patch to work on try (2.0)

>--- a/testing/mochitest/redirect.js
>+++ b/testing/mochitest/redirect.js
>-  webNav.loadURI(aURL + location.search,
>+  SpecialPowers.loadURI(window, aURL + location.search,
>                  null, null, null, null);

Could have reindented this.