Closed Bug 1586264 Opened 5 years ago Closed 5 years ago

Only import the used objects with ChromeUtils.import

Categories

(Chat Core :: General, task)

Product:
Chat Core
Component:
General
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Instantbird 71

People

(Reporter: clokep, Assigned: clokep)

References

Details

Attachments

(1 file)

Patch v1
(deleted), patch
aleca
: review+
Details | Diff | Splinter Review

Bug 1520643 added all imports to ChromeUtils.import() when only some are unnecessary. This strips the necessary imports down to only what is used in each file.

Attached patch Patch v1 (deleted) — — Splinter Review

This patch does two things:

  1. Removes all the unnecessary imports.
  2. Removes some uses of Services.scriptloader.loadSubScript which were being used to import publicly exported objects.

To test this I:

  1. Connected an IRC account.
  2. Ensure messages could be sent and received.
  3. Ensured commands run properly (type /whois aleca)
  4. Ensured an XMPP account still worked properly.
Attachment #9098824 - Flags: review?(alessandro)

I should mention that I found unused imports by just searching for each import with my editor on a file-by-file basis.

Comment on attachment 9098824 [details] [diff] [review] Patch v1 Review of attachment 9098824 [details] [diff] [review]: ----------------------------------------------------------------- Everything works perfectly for both XMPP and IRC accounts. I don't know if you're aware, but I got a bunch of console errors while using it. I didn't notice any issue in the client, tho. ``` JavaScript error: file:///home/alecaddd/Mozilla/source/obj-x86_64-pc-linux-gnu/dist/bin/components/imIncomingServer.js, line 163: TypeError: this.imAccount is null JavaScript error: chrome://chat/content/conversation-browser.js, line 425: TypeError: this.docShell is null prpl-irc: Failed to set character set to: false for aleca@irc.mozilla.org. JavaScript error: file:///home/alecaddd/Mozilla/source/obj-x86_64-pc-linux-gnu/dist/bin/components/imIncomingServer.js, line 163: TypeError: this.imAccount is null prpl-irc: Failed to convert CAP LS from Unicode to false. prpl-irc: Failed to convert NICK aleca from Unicode to false. prpl-irc: Failed to convert USER Thunderbird 0 * aleca from Unicode to false. prpl-irc: Failed to convert CAP REQ multi-prefix from Unicode to false. prpl-irc: Failed to convert NICK aleca1 from Unicode to false. prpl-irc: Failed to convert CAP END from Unicode to false. prpl-irc: Failed to convert PROTOCTL NAMESX from Unicode to false. prpl-irc: Failed to convert WATCH C from Unicode to false. prpl-irc: Failed to convert JOIN #test from Unicode to false. prpl-irc: Failed to convert MODE #test from Unicode to false. prpl-irc: Failed to convert PRIVMSG #test asd from Unicode to false. prpl-irc: Failed to convert WHOIS aleca from Unicode to false. prpl-irc: Failed to convert WHOIS aleca from Unicode to false. prpl-irc: Failed to convert WHOIS aleca1 from Unicode to false. ```
Attachment #9098824 - Flags: review?(alessandro) → review+

(In reply to Alessandro Castellani (:aleca) from comment #3)

prpl-irc: Failed to convert CAP LS
from Unicode to false.

Do you get any of these errors on master? That seems very suspicious that something changed in character encoding handling... Shouldn't be related to this patch, but I'll do some more testing.

Yes, I get those errors on trunk without your patch applied.
This should be good to go.

Did you already do a try run just to be sure no test is failing?

(In reply to Alessandro Castellani (:aleca) from comment #5)

Yes, I get those errors on trunk without your patch applied.
This should be good to go.

Can you file a separate issue about that please? I suspect that's a bustage...

Did you already do a try run just to be sure no test is failing?

I ran the chat tests manually.

Pushed by clokep@gmail.com:
https://hg.mozilla.org/comm-central/rev/a094c4ec89ae
Only import the used objects with ChromeUtils.import. r=aleca

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED

I filed bug 1587054 about the errors converting from/to Unicode.

Target Milestone: --- → Instantbird 71
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: