Closed Bug 1562314 Opened 1 year ago Closed 1 year ago

Creating a new Chat account loses port number

Categories

(Thunderbird :: Instant Messaging, defect)

x86_64
All
defect
Not set
normal

Tracking

(thunderbird68+ fixed, thunderbird69 fixed)

RESOLVED FIXED
Thunderbird 69.0
Tracking Status
thunderbird68 + fixed
thunderbird69 --- fixed

People

(Reporter: walts48, Assigned: aceman)

References

Details

(Keywords: regression)

Attachments

(1 file)

While testing TB 68.0b3 rc (Buid1) with a fresh test profile on Windows 10 and Ubuntu 18.04.2.

Click Chat to create a new Chat account
Select IRC
Click Next
Enter the Username
Enter irc.mozilla.org for the Server
Click Next
Enter a Password
In the Advanced Options dialog, note that the Port is correct for having SSL selected
Click Finish to create the account
Note that in the Chat Account Wizard Summary the Port: is shown as false
Try connecting and you can't.

Checked the Error Console
"TypeError: this.imAccount is null
imIncomingServer.js:143:5
Connection closed by server. 5 irc.js:7"

Check the account in Account settings and see the Port is set to 0

Set the Port to the correct number for SSL enabled and can connect successfully.

No problem when updating from 68.0b2 to 68.0b3 using the already configured test profiles for b2.

https://dxr.mozilla.org/comm-beta/source/mail/components/im/imIncomingServer.js#143

Alice, can you please find the regression for us. I could easily reproduce it.

Keywords: regression

OK, when you look at the port in the account settings, you see 0 instead of 6697. If you change that manually to 6697, you can connect. So something went wrong in IM account creation.

Flags: needinfo?(alice0775)

I do not have any Username/Password for irc.
What should I do?

Flags: needinfo?(alice0775)

Invent something, it doesn't matter for account creation and the fact that the port is lost. Use alice/12345.

Thanks so much, Alice, that makes the search easier. Likely bug 1556868 again which already caused a few regressions.

Attached patch 1562314.patchSplinter Review

The bug is at https://searchfox.org/comm-central/source/mail/components/im/content/imAccountWizard.js#366, it seems "checked" attribute exists on all <input> elements, regardless of type. So we were taking that branch, returning a value of 'false' for the 'port' box, ignoring the real port number that was in the "value" property.

Assignee: nobody → acelists

(In reply to WaltS48 [:walts48] from comment #0)

"TypeError: this.imAccount is null
imIncomingServer.js:143:5
Connection closed by server. 5 irc.js:7"

This one is caused by trying to save the pref with name "canChangeStoreType" as bool in an imAccount.
This part is not fixed by my patch, please investigate further.

Blocks: 1562438

The JS message is a false lead, it already exists since TB 60, see bug 1562438.

The issue at hand is that the port number is lost.

Summary: TypeError: this.imAccount is null imIncomingServer.js:143:5 when creating a new Chat account → Creating a new Chat account looses port number
Comment on attachment 9074984 [details] [diff] [review]
1562314.patch

Thanks. On trunk it still doesn't connect automatically, but at least the port number is stored correctly in the account now. Let's see on beta whether this is all we need.
Attachment #9074984 - Flags: review+
Target Milestone: --- → Thunderbird 69.0
Summary: Creating a new Chat account looses port number → Creating a new Chat account loses port number

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/b7fc532744f4
read element.checked only if the <input> element is a checkbox in IM account wizard. r=jorgk DONTBUILD

Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED

Well, as I said in comment #10, this still doesn't connect automatically any more.
I use all the default values: New profile, add chat account, IRC, xxx/chat.freenode.net, 3x Next, 1x Finish. Doesn't connect, but TB 60 does.

Before this change, I believe, it tried to connect, but failed, since the port was 0.

I debugged this a bit and at the line we changed saw types of blank, "number" and "password", I didn't see "checkbox".

dump(`=== ${elt.getAttribute("type")} ${("checked" in elt)} ${aId}\n`);

gives
=== password false password
=== false alias
=== number true prpl-irc-port
=== true prpl-irc-ssl
=== false prpl-irc-encoding
=== false prpl-irc-quitmsg
=== false prpl-irc-partmsg
=== true prpl-irc-showServerTab
=== false prpl-irc-alternateNicks
=== true connectNow

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/7302ce3d1e78
Follow-up: Check for type number explicitly. r=me DONTBUILD

(In reply to Jorg K (GMT+2) from comment #13)

I debugged this a bit and at the line we changed saw types of blank, "number" and "password", I didn't see "checkbox".

True, the dialog uses <checkbox> for checkboxes, not <input type="checkbox">, so my patch fixed the type="number" boxes, but may have broken checkboxes :(

(In reply to Pulsebot from comment #15)

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/7302ce3d1e78
Follow-up: Check for type number explicitly. r=me DONTBUILD

r=aceman for this fix, thanks :)

Blocks: 1591148
You need to log in before you can comment on or make changes to this bug.