Closed Bug 1586264 Opened 5 months ago Closed 5 months ago

Only import the used objects with ChromeUtils.import

Categories

(Chat Core :: General, task)

task
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Instantbird 71

People

(Reporter: clokep, Assigned: clokep)

References

Details

Attachments

(1 file)

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 v1Splinter 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 months 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.