After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 720706 - userList: Don't crash if not channel for room
userList: Don't crash if not channel for room
Status: RESOLVED FIXED
Product: polari
Classification: Applications
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: Polari maintainers
Polari maintainers
Depends on:
Blocks:
 
 
Reported: 2013-12-18 23:16 UTC by Carlos Soriano
Modified: 2014-02-28 13:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
userList: Don't crash if not channel for room (1.18 KB, patch)
2013-12-18 23:16 UTC, Carlos Soriano
none Details | Review
userList: Don't crash if not channel for room (1.18 KB, patch)
2013-12-18 23:30 UTC, Carlos Soriano
none Details | Review
userList: Don't crash if not channel for room (1.20 KB, patch)
2013-12-18 23:33 UTC, Carlos Soriano
reviewed Details | Review
userList: Don't update header if not channel for room (953 bytes, patch)
2014-02-28 13:01 UTC, Carlos Soriano
none Details | Review
userList: Don't update header if not channel (944 bytes, patch)
2014-02-28 13:02 UTC, Carlos Soriano
committed Details | Review

Description Carlos Soriano 2013-12-18 23:16:17 UTC
Since we can have rooms withouth channel, trying to
count the number uf users ifnot channel is set for the
room causes a crash.
Just ignore the count if room doesn't have channel.
Comment 1 Carlos Soriano 2013-12-18 23:16:19 UTC
Created attachment 264505 [details] [review]
userList: Don't crash if not channel  for room
Comment 2 Carlos Soriano 2013-12-18 23:30:21 UTC
Created attachment 264507 [details] [review]
userList: Don't crash if not channel for room

Since we can have rooms withouth channel, trying to
count the number uf users if not channel is set for the
room causes a crash.
Just ignore the count if room doesn't have channel.
Comment 3 Carlos Soriano 2013-12-18 23:33:47 UTC
Created attachment 264508 [details] [review]
userList: Don't crash if not channel for room

Since we can have rooms withouth channel, trying to
count the number of users if not channel is set for the
room causes a crash.
Just ignore the count calculation if room doesn't have channel.
Comment 4 Florian Müllner 2014-02-21 18:40:38 UTC
Review of attachment 264508 [details] [review]:

Sorry for the late review - my gut reaction was "why are we trying to update that count anyway in that case?". Apparently we get this error, but I was a bit lazy of looking into it :-)
So while the user list will be closed and disabled when the connection is lost, we will also clear out the list box in _onChannelChanged() - which then triggers the header function to run. So yeah, we should get rid of those warnings. However I don't see much of a point in setting the label to anything - after all, we will never show it. So I'd suggest to either return early from the header function when this._room.channel is unset, or set/unset the header function itself in _onChannelChanged().
Comment 5 Carlos Soriano 2014-02-28 13:01:26 UTC
Created attachment 270558 [details] [review]
userList: Don't update header if not channel for room

Since we can have rooms withouth channel, trying to
count the number of users if not channel is set for the
room causes a warning.
Just ignore don't add the header if there's not channel for room.
Comment 6 Carlos Soriano 2014-02-28 13:02:37 UTC
Created attachment 270559 [details] [review]
userList: Don't update header if not channel

Since we can have rooms withouth channel, trying to
count the number of users if not channel is set for the
room causes a warning.
Just ignore don't add the header if there's not channel for room.
Comment 7 Florian Müllner 2014-02-28 13:26:30 UTC
Review of attachment 270559 [details] [review]:

Some nits in the commit message:
- withouth -> without
- if not channel -> if no channel
- ignore don't add -> "ignore" or "don't add" (the former is slightly more correct, as we leave whatever header was set before and just ignore any updates)
- not channel for room -> no channel for the room

Fine to push with a fixed message.
Comment 8 Carlos Soriano 2014-02-28 13:49:47 UTC
(In reply to comment #7)
> Review of attachment 270559 [details] [review]:
> 
> Some nits in the commit message:
> - withouth -> without
> - if not channel -> if no channel
> - ignore don't add -> "ignore" or "don't add" (the former is slightly more
> correct, as we leave whatever header was set before and just ignore any
> updates)
> - not channel for room -> no channel for the room
> 
> Fine to push with a fixed message.

Sorry, I'll try to be more cautious with such errors.