#978 Trunk MUC does not carry error into occupant leave status
Reporter
MattJ
Owner
MattJ
Created
Updated
Stars
★★ (2)
Tags
Status-Fixed
Type-Defect
Priority-Medium
MattJ
on
When a MUC occupant is kicked due to an error, Prosody 0.9 and 0.10 show the user leaving with a status like "Kicked: service-unavailable" (depending on the error condition and text).
In trunk, this does not happen and it looks like the user left normally.
This is due to a couple of things:
In room:publicise_occupant_status():
if pr and (pr.attr.type ~= "unavailable" and occupant.role ~= nil) then
Translates to: "if the occupant has presence, and it is not 'unavailable' and the occupant has a role in the room". Following this logic, the function chooses to always create a new unavailable presence, and will never use an unavailable presence that the occupant sent to the room.
This code was introduced in 5032d4817a30 and the logic was later modified in e49737de9872.
Neither of these seem to work, however, as even before e49737de9872's changes, occupant.role is not nil (which I assume is an attempt to catch occupants that are no longer "in" the room).
occupant.role should be nil for an occupant that has left the room, but due to the way room:save_occupant() only modifies a copy of the original occupant, the input 'occupant' object is always unmodified - therefore still has occupant.role set when room:handle_kickable() passes it on to room:publicise_occupant_status().
Possibly room:save_occupant() should return the copied and modified occupant object.
When a MUC occupant is kicked due to an error, Prosody 0.9 and 0.10 show the user leaving with a status like "Kicked: service-unavailable" (depending on the error condition and text). In trunk, this does not happen and it looks like the user left normally. This is due to a couple of things: In room:publicise_occupant_status(): if pr and (pr.attr.type ~= "unavailable" and occupant.role ~= nil) then Translates to: "if the occupant has presence, and it is not 'unavailable' and the occupant has a role in the room". Following this logic, the function chooses to always create a new unavailable presence, and will never use an unavailable presence that the occupant sent to the room. This code was introduced in 5032d4817a30 and the logic was later modified in e49737de9872. Neither of these seem to work, however, as even before e49737de9872's changes, occupant.role is not nil (which I assume is an attempt to catch occupants that are no longer "in" the room). occupant.role should be nil for an occupant that has left the room, but due to the way room:save_occupant() only modifies a copy of the original occupant, the input 'occupant' object is always unmodified - therefore still has occupant.role set when room:handle_kickable() passes it on to room:publicise_occupant_status(). Possibly room:save_occupant() should return the copied and modified occupant object.
Fixed in a971023e9b6e
Changes