Closed
Bug 284149
Opened 19 years ago
Closed 19 years ago
A lack of secure servers on a network when using ircs:// urls should give a better error message
Categories
(Other Applications :: ChatZilla, defect)
Other Applications
ChatZilla
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: Gijs, Assigned: rginda)
Details
(Whiteboard: [cz-patch][cz-0.9.68])
Attachments
(1 file, 2 obsolete files)
13.08 KB,
patch
|
rginda
:
review+
asa
:
approval1.8b2+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.6) Gecko/20050223 Firefox/1.0.1 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.6) Gecko/20050223 Firefox/1.0.1 When connecting to a network using /attach ircs://foonet/, while the network does not have any servers capable of making a secure connection returns "Connection attempts exhausted, giving up", which is non-descriptive of the actual problem (there's not an ssl-enabled server in the network, as far as ChatZilla knows) Reproducible: Always Steps to Reproduce: 1. /attach ircs://moznet/ 2. Get a weird message about connection attempts being exhausted, without any notice of any connection attempts being actually tried / made. 3. Get confused Actual Results: I did not know what happened to cause the connection to fail, and had to resort to pulling apart the javascript code behind ChatZilla to find out ;-). Expected Results: It should have displayed a message that the network did not have any servers capable of making a secure connection.
Updated•19 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Reporter | ||
Comment 1•19 years ago
|
||
Fixes: - "DISCONNECT" message type. Uses that instead of "INFO" for when the user disconnects (instead of a non-intended disconnect). - Updated all motifs in mozilla/extensions/irc/xul/skin to style "DISCONNECT" in the same way they style "ERROR", to make it stand out as it used to (since originally the message type was actually "ERROR"). - Adds the secure version of the irc.mozilla.org server to the moznet irc network, so ircs://moznet will actually work :-). (also fixes a 80-character a line style break in the definition of the networks) When trying to connect to a network using a secure connection: - A better error message is used when ChatZilla doesn't know any servers in the network that support a secure connection - "Connection Attempts" to non-secure servers are no longer counted as real connection attempts, so when ChatZilla uses up all 5 connection attempts it actually tried to connect to secure servers 5 times, not just to all servers (of which some may NOT have supported a secure connection, thus failing instantly). This should solve all problems previously reported (see comment 15 on bug 281706) Requesting review from rginda.
Attachment #175945 -
Flags: review?(rginda)
Assignee | ||
Comment 2•19 years ago
|
||
Comment on attachment 175945 [details] [diff] [review] Patch v1 I'd suggest factoring out the "has secure server" check from CIRCNetwork.prototype.connect into a new hasSecureServer() method. Someone will probably find a reason to use it later. This is a bit awkward: A secure connection could not be established because there are no known secure servers to connect to. How about: The network %S has no secure servers defined. In the properties file: msg.rsp.disc = [QUIT] "disc" is a bit ambiguous, spelling it out isn't so bad. Other than that it looks good to me.
Reporter | ||
Comment 3•19 years ago
|
||
I also fixed a strict warning now thrown for net_connect, which did not always return a value :-).
Reporter | ||
Updated•19 years ago
|
Attachment #175945 -
Attachment is obsolete: true
Attachment #176139 -
Flags: review?(rginda)
Reporter | ||
Updated•19 years ago
|
Attachment #175945 -
Flags: review?(rginda)
Assignee | ||
Comment 4•19 years ago
|
||
Comment on attachment 176139 [details] [diff] [review] Patch incorporating the changes requested Ah, now it's easier to see that you don't need to finish the loop after you find the first secure server. You can safely return true the first time you find one that is secure. You can even replace the loacal "hasSecure" with a constant "return false" at the end.
Reporter | ||
Comment 5•19 years ago
|
||
I'll just keep trying ;). Done the change you wanted, I believe :).
Attachment #176139 -
Attachment is obsolete: true
Attachment #176169 -
Flags: review?(rginda)
Reporter | ||
Updated•19 years ago
|
Attachment #176139 -
Flags: review?(rginda)
Assignee | ||
Comment 6•19 years ago
|
||
Comment on attachment 176169 [details] [diff] [review] patch v3 (Bail out upon finding a secure server) r=rginda
Attachment #176169 -
Flags: review?(rginda) → review+
Reporter | ||
Updated•19 years ago
|
Whiteboard: cz-patch
Updated•19 years ago
|
Attachment #176169 -
Flags: approval1.8b2?
Comment 7•19 years ago
|
||
Comment on attachment 176169 [details] [diff] [review] patch v3 (Bail out upon finding a secure server) a=asa
Attachment #176169 -
Flags: approval1.8b2? → approval1.8b2+
Comment 8•19 years ago
|
||
Checked in --> FIXED. Automated builds at http://twpol.dyndns.org/mozilla/chatzilla/nightly/ will include this shortly.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Updated•19 years ago
|
Whiteboard: cz-patch → [cz-patch][cz-0.9.68]
You need to log in
before you can comment on or make changes to this bug.
Description
•