1206238 - Improve XMPP handleErrors function to display error system messages more easily

Status: RESOLVED FIXED

(Chat Core :: XMPP, defect)

Description

[aleth [:aleth]]

Now there's quite a bit of error handling in XMPP, it's clear the most common case of an error handling function is one that simply displays a localised error system message. We should be able to remove a lot of code duplication by adding this case to  handleErrors (the error-handler-callback-generating helper function).

Comment 1

[aleth [:aleth]]

Attachment: errorhandling.diff

See amended comment above handleErrors() in the patch for details.

Suggestions for improvements/amendments welcome!

Comment 2

[aleth [:aleth]]

Attachment: errorhandling.diff v2

A bit more readable, I think.

Comment 3

[Patrick Cloke [:clokep]]

Comment on attachment 8663140 [details] [diff] [review]
errorhandling.diff v2

Review of attachment 8663140 [details] [diff] [review]:
-----------------------------------------------------------------

Minor nits. Thanks for cleaning this up!

::: chat/protocols/xmpp/xmpp.jsm
@@ +481,2 @@
>        // TODO: We should then discover user's reserved nickname (it could be
> +      // discovered before joining a room): XEP-0045 (7.12): Discovering

Was this comment purposefully wrapped this way? This sentence seems to have three colons in it. I find that confusing. :)

@@ +1396,1 @@
>    handleErrors(aHandlers, aThis) {

Can we add some (vertical) white space into this function? It looks like there's three separate sections (each separated by a return false). (I'd also like a brief comment that reiterates the block comment about what each section is doing. :))

Comment 4

[aleth [:aleth]]

Attachment: errorhandling.diff v3

Nits fixed and inlined some helper vars.

Comment 5

[Patrick Cloke [:clokep]]

Comment on attachment 8664559 [details] [diff] [review]
errorhandling.diff v3

Review of attachment 8664559 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 6

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/a5679881ec4c40a1e2441ee27cb5d07db70c9a5e
Bug 1206238 - Improve XMPP handleErrors to display error system messages directly. r=clokep