Closed Bug 955040 Opened 10 years ago Closed 10 years ago

Display a correct error message in the conversation when receiving a message type=error stanza

Categories

(Chat Core :: XMPP, defect)

defect
Not set
trivial

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: florian, Assigned: florian)

References

Details

(Whiteboard: [1.3-wanted])

Attachments

(1 file)

*** Original post on bio 1610 at 2012-07-31 10:50:00 UTC ***

Bug 954999 (bio 1568) added support for displaying an error message when receiving a message type=error stanza, but as 1.2 is already string frozen, we couldn't include a good error message and had to reuse an existing message.

I think we would want the error message displayed to the user to mention which message couldn't be delivered.
Blocks: 954999
Whiteboard: [1.3-wanted]
Attached patch Untested patchSplinter Review
*** Original post on bio 1610 as attmnt 2039 at 2012-11-04 11:03:00 UTC ***

Note: I haven't tested this at all (but it seems straight forward).

The only thing I wondered was if we should be extra careful, and test that aMsg isn't the empty string, and if it is, either drop the error completely, or fallback to connection.error.incorrectResponse. I'm not sure if this can really happen.

Other possible debatable points: should we include a line break before the original message? Should we display the original messages in quotes?
For the quotes I tend to think "no", as the end quote would look odd several lines later (if the original message was long). For the line break, I'm not sure, but for a short message I think it wouldn't look great.
Attachment #8353799 - Flags: review?(clokep)
Assignee: nobody → florian
Comment on attachment 8353799 [details] [diff] [review]
Untested patch

*** Original change on bio 1610 attmnt 2039 at 2012-11-05 02:21:08 UTC ***

(In reply to comment #1)
> Created attachment 8353799 [details] [diff] [review] (bio-attmnt 2039) [details]
> Untested patch
> 
> Note: I haven't tested this at all (but it seems straight forward).
Please test this before checking in? :)

> The only thing I wondered was if we should be extra careful, and test that aMsg
> isn't the empty string, and if it is, either drop the error completely, or
> fallback to connection.error.incorrectResponse. I'm not sure if this can really
> happen.
That seems like a weird situation. I don't like optimizing for a situation we aren't sure if it can happen though.

> Other possible debatable points: should we include a line break before the
> original message? For the line break, I'm not
> sure, but for a short message I think it wouldn't look great.
I don't find this necessary and I agree I think it could look bad in certain situations. Let's not over think this, just leave it out.

> Should we display the original messages in quotes?
> For the quotes I tend to think "no", as the end quote would look odd several
> lines later (if the original message was long). 
No quotes please, I think the : is obvious enough that the rest is part of a message.
Attachment #8353799 - Flags: review?(clokep) → review+
*** Original post on bio 1610 at 2012-11-05 23:06:50 UTC ***

(In reply to comment #2)

> > Note: I haven't tested this at all (but it seems straight forward).
> Please test this before checking in? :)

Done:

00:04:43 - InstantbirdTest: blah blah blah
00:04:43 - This message could not be delivered: blah blah blah
*** Original post on bio 1610 at 2012-11-06 10:35:08 UTC ***

http://hg.instantbird.org/instantbird/rev/a9b29651dcf2
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.3
You need to log in before you can comment on or make changes to this bug.