Closed Bug 922008 Opened 6 years ago Closed 6 years ago

Remove more enablePrivilege calls, part2

Categories

(Testing :: Mochitest, defect)

x86
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla27

People

(Reporter: martijn.martijn, Assigned: martijn.martijn)

Details

Attachments

(1 file, 1 obsolete file)

I think this is the last part, where it's relatively easy to remove some enablePrivilege calls.

I pushed this to try: https://tbpl.mozilla.org/?tree=Try&rev=5734cdabae20
Attached patch enablepriv.diff (obsolete) — Splinter Review
Comment on attachment 811933 [details] [diff] [review]
enablepriv.diff

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

Try server seems to be going well.

I removed browser/base/content/test/general/test_bug452451.html in this patch. The test isn't run at all and the javascript.options.relimit pref doesn't seem to exist anymore.
I also removed dom/tests/mochitest/bugs/iframe_bug534362.html, because that seems to be a left-over from some test that doesn't exist anymore.
I also removed testing/mochitest/tests/test_bug366645.xhtml. The test isn't being run. I tried to run it, but got failures with @mozilla.org/url-classifier/ not existing or something like that. I think the test is completely outdated.
The same goes for toolkit/components/url-classifier/content/enchash-decrypter.js and toolkit/components/url-classifier/tests/test_enchash-decrypter.xhtml. They are not being run now and they look outdated. But perhaps someone who knows about url-classifier should look at these tests to see if they are still potentially worthwhile?

Please check if the changes in content/base/test/test_bug498897.html and content/xml/document/test/test_bug392338.html are acceptable.

I also made changes to bug346659 files, but there is still an enablePrivilege call necessary somewhere, so perhaps I shouldn't be making these changes. This test should perhaps be converted into a chrome mochitest.

toolkit/content/tests/chrome/test_cursorsnap.xul is currently not running at all, fwiw.
Attachment #811933 - Flags: review?(jmaher)
Comment on attachment 811933 [details] [diff] [review]
enablepriv.diff

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

only a few nits.

::: dom/tests/mochitest/bugs/bug346659-opener.html
@@ +8,3 @@
>          window.testNum = cmd.load;
>        } else if ("write" in cmd) {
> +        sessionStorage.setItem("testNum", cmd.write);

why are we adding sessionStorage in here?  It seems as though we could be testing the wrong thing.

::: dom/tests/mochitest/bugs/bug346659-parent.html
@@ +10,3 @@
>          window.testNum = cmd.load;
>        } else if ("write" in cmd) {
> +        sessionStorage.setItem("testNum", cmd.write);

more sessionStorage stuff here.

::: testing/mochitest/tests/test_bug366645.xhtml
@@ -1,3 @@
> -<html xmlns="http://www.w3.org/1999/xhtml">
> -<!--
> -https://bugzilla.mozilla.org/show_bug.cgi?id=366645

this is referenced in testing/mochitest/tests/index.html.  Can we remove the reference.
Attachment #811933 - Flags: review?(jmaher) → review+
(In reply to Joel Maher (:jmaher) from comment #3)
> why are we adding sessionStorage in here?  It seems as though we could be
> testing the wrong thing.

I'll revert the changes to the bug346659 files. There is not much value to them anyway, since I couldn't get rid of the last enablePrivilege call.
I need to figure out a better solution for this.
Updated patch.

(In reply to Joel Maher (:jmaher) from comment #3)
> this is referenced in testing/mochitest/tests/index.html.  Can we remove the
> reference.

I did that in this patch, I also did what I said in comment 4.
Attachment #811933 - Attachment is obsolete: true
Attachment #812249 - Attachment description: 922008.diff → 922008.diff (for check-in)
Keywords: checkin-needed
Assignee: nobody → martijn.martijn
https://hg.mozilla.org/mozilla-central/rev/0ed8e884d352
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.