1793227 - ESlint fixes for js/xpconnect/ subdirectories tests/chrome/, tests/browser/ and js/xpconnect/loader/

Status: RESOLVED FIXED

(Core :: XPConnect, task, P3)

Description

[Andrew McCreight [:mccr8]]

I started looking at this, and it seems tractable.

Comment 1

[Andrew McCreight [:mccr8]]

js/xpconnect/loader/ only has a few issues that need to be manually fixed, mostly related to ignoring weird stuff XPCOMUtils does that we don't want to allow in other places, so I'll fix that, too.

Comment 3

[Andrew McCreight [:mccr8]]

Attachment: Bug 1793227, part 1 - Disable generated QI errors in two places.

Disable this so the autofix doesn't erroneously change them.

Comment 4

[Andrew McCreight [:mccr8]]

Attachment: Bug 1793227, part 2 - Manually convert some http:// URLs to https:// in chrome XPConnect tests.

The eslint autofixer makes URLs that appear in strings in JS into https://
URLs. However, it does not change any URLs in the HTML part of the tests, which
caused some tests to fail. To try to ensure that the autofixer didn't break
tests in any way that doesn't show up as a failure, I did a light audit of the
tests. Specifically, I looked at files in js/xpconnect/tests/chrome/ containing
the string "example." in a URL string in more than one place. I switched these
URLs over to https://, both in JS and HTML.

I left alone test_secureContexts.html, which uses an http:// URL in HTML, but
is clearly trying to test http://.

I did a similar check for mochi.test but there weren't any tests that used it
more than once.

The tests that originally were broken by the autofix were:

• test_evalInWindow.xhtml
• test_expandosharing.xhtml
• test_wrappers.xhtml
• test_xrayic.xhtml

My audit didn't find any other instances that looked like they might fail. I
did fix a few other places, but I think in those cases the HTML and JS URLs were
cross origin anyways so it shouldn't really matter if one is https:// and the
other is http://.

I also changed single quotes to double quotes for the URLs I changed, because
the autofix is going to do that anyways.

Comment 5

[Andrew McCreight [:mccr8]]

Attachment: Bug 1793227, part 3 - Automatic fixes for js/xpconnect/tests/chrome.

Also change .eslintignore to cover the other directories.

This also fixes the indentation the automatic fixer messed up in a few XHTML files.

Comment 6

[Andrew McCreight [:mccr8]]

Attachment: Bug 1793227, part 4 - Manual eslint fixes for js/xpconnect/tests/chrome.

Comment 7

[Andrew McCreight [:mccr8]]

Attachment: Bug 1793227, part 6 - Automated fixes for XPCOMUtils.sys.mjs.

Comment 8

[Andrew McCreight [:mccr8]]

Attachment: Bug 1793227, part 7 - Manual fixes for XPCOMUtils.sys.mjs.

Mostly this is adding some ignores for XPCOMUtils doing weird stuff
we don't want to allow other places, but that I assume is okay here
for now.

Also, one place was missing an explicit return.

Comment 9

[Andrew McCreight [:mccr8]]

Attachment: Bug 1793227, part 8 - Manual eslint fixes for js/xpconnect/tests/browser/.

browser_exception_leak.js is intentionally creating a leak, so maybe the unused
doc is intentional.

browser_import_mapped_jsm.js is deliberately testing the behavior of Cu.import,
so ignore the failure there.

Comment 10

[Andrew McCreight [:mccr8]]

Attachment: Bug 1793227, part 9 - Automated fixes for js/xpconnect/tests/browser/.

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The following patch is waiting for review from a reviewer who resigned from the review:

ID	Title	Author	Reviewer Status
D158506	Bug 1793227, part 6 - Manual fixes for XPCOMUtils.sys.mjs.	mccr8	kmag: Resigned from review

:mccr8, could you please find another reviewer?

For more information, please visit auto_nag documentation.

Comment 12

[Andrew McCreight [:mccr8]]

I need to update the patch first.

Comment 13

[Andrew McCreight [:mccr8]]

Attachment: Bug 1793227, part 5 - Clean up some named function cruft in XPCOMUtils.sys.mjs.

This used to be needed to improve the debugging information, but isn't any more.

I only did minimal fixing up of the whitespace, because that'll be done properly
automatically in the next patch.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4c3f5d1c4a1a
part 1 - Disable generated QI errors in two places. r=kmag
https://hg.mozilla.org/integration/autoland/rev/a28388486cd4
part 2 - Manually convert some http:// URLs to https:// in chrome XPConnect tests. r=kmag
https://hg.mozilla.org/integration/autoland/rev/49dc1e5a8419
part 3 - Automatic fixes for js/xpconnect/tests/chrome. r=kmag
https://hg.mozilla.org/integration/autoland/rev/d0ee3b2c51c0
part 4 - Manual eslint fixes for js/xpconnect/tests/chrome. r=kmag
https://hg.mozilla.org/integration/autoland/rev/a93cc88e3269
part 5 - Clean up some named function cruft in XPCOMUtils.sys.mjs. r=kmag
https://hg.mozilla.org/integration/autoland/rev/dcc220ecce10
part 6 - Automated fixes for XPCOMUtils.sys.mjs. r=kmag
https://hg.mozilla.org/integration/autoland/rev/a52b2e23d137
part 7 - Manual fixes for XPCOMUtils.sys.mjs. r=kmag
https://hg.mozilla.org/integration/autoland/rev/ce843aa67aa0
part 8 - Manual eslint fixes for js/xpconnect/tests/browser/. r=kmag
https://hg.mozilla.org/integration/autoland/rev/939c9c30c83c
part 9 - Automated fixes for js/xpconnect/tests/browser/. r=kmag

Comment 15

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/4c3f5d1c4a1a
https://hg.mozilla.org/mozilla-central/rev/a28388486cd4
https://hg.mozilla.org/mozilla-central/rev/49dc1e5a8419
https://hg.mozilla.org/mozilla-central/rev/d0ee3b2c51c0
https://hg.mozilla.org/mozilla-central/rev/a93cc88e3269
https://hg.mozilla.org/mozilla-central/rev/dcc220ecce10
https://hg.mozilla.org/mozilla-central/rev/a52b2e23d137
https://hg.mozilla.org/mozilla-central/rev/ce843aa67aa0
https://hg.mozilla.org/mozilla-central/rev/939c9c30c83c