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 747415 - Encoding Issues with the Topic
Encoding Issues with the Topic
Status: RESOLVED FIXED
Product: polari
Classification: Applications
Component: general
3.14.x
Other Linux
: Normal normal
: ---
Assigned To: Polari maintainers
Polari maintainers
Depends on:
Blocks:
 
 
Reported: 2015-04-06 15:17 UTC by patdvkc
Modified: 2016-02-10 10:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Encoding issues in the topic. (16.14 KB, image/png)
2015-04-06 15:17 UTC, patdvkc
  Details
room: Strip color codes from room topics (1.40 KB, patch)
2016-02-09 19:05 UTC, Kunaal Jain
none Details | Review
room: Strip color codes from room topics (1.48 KB, patch)
2016-02-09 20:38 UTC, Kunaal Jain
none Details | Review
room: Strip color codes from room topics (1.48 KB, patch)
2016-02-09 21:03 UTC, Kunaal Jain
committed Details | Review

Description patdvkc 2015-04-06 15:17:22 UTC
Created attachment 301026 [details]
Encoding issues in the topic.

I'm not exactly sure what's causing this, but whenever there are some special symbols (or could it be colored titles?) in the topic, encoding issues such as these in the attachment appear.
Comment 1 Florian Müllner 2015-04-06 15:32:58 UTC
Yeah, those look like color codes ...
Comment 2 Bastian Ilsø 2016-02-02 17:14:11 UTC
Bug 711081 already provides a fix for status messages, we just need to apply it to the topic too.
Comment 3 Kunaal Jain 2016-02-09 19:05:07 UTC
Created attachment 320738 [details] [review]
room: Strip color codes from room topics

Topic can be set by user, and some IRC clients
support color codes. We don't support color codes,
so displaying escape sequences looks broken.
We already strip color codes for status messages
(bug 711081), do the same for room topics.
Comment 4 Florian Müllner 2016-02-09 19:19:55 UTC
Review of attachment 320738 [details] [review]:

::: src/lib/polari-room.c
@@ +380,1 @@
   if (subject == NULL || g_strcmp0 (priv->topic, subject) == 0)

You leak subject if that condition is met.
Comment 5 Kunaal Jain 2016-02-09 20:38:56 UTC
Created attachment 320753 [details] [review]
room: Strip color codes from room topics

Topic can be set by user, and some IRC clients
support color codes. We don't support color codes,
so displaying escape sequences looks broken.
We already strip color codes for status messages
(bug 711081), do the same for room topics.
Comment 6 Florian Müllner 2016-02-09 20:50:42 UTC
Review of attachment 320753 [details] [review]:

Looks good, just some style nits

::: src/lib/polari-room.c
@@ +371,3 @@
+  raw_subject = tp_asv_get_string (properties, "Subject");
+
+  if (raw_subject)

I think this is slightly cleaner:

raw_subject = ...
if (raw_subject == NULL)
  return;

subject = strip_color_codes();
if (g_strcmp0 (topic, subject) == 0)
 ...

@@ +381,2 @@
   g_free (priv->topic);
+  priv->topic = g_strdup (subject);

We already own the memory for subject here (i.e. the return value of strip_color_codes()), so we could just use that directly (i.e. no strdup()/free())
Comment 7 Kunaal Jain 2016-02-09 21:03:09 UTC
Created attachment 320754 [details] [review]
room: Strip color codes from room topics

Topic can be set by user, and some IRC clients
support color codes. We don't support color codes,
so displaying escape sequences looks broken.
We already strip color codes for status messages
(bug 711081), do the same for room topics.
Comment 8 Florian Müllner 2016-02-09 21:09:45 UTC
Review of attachment 320754 [details] [review]:

OK
Comment 9 Kunaal Jain 2016-02-10 04:25:03 UTC
Comment on attachment 320754 [details] [review]
room: Strip color codes from room topics

Attachment 320754 [details] pushed as 5944ead - room: Strip color codes from room topics