Bug 178420 - Low battery message shows HTML codes
Summary: Low battery message shows HTML codes
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: notify-daemon
Version: 5
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Christopher Aillon
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks: FC5Target
TreeView+ depends on / blocked
 
Reported: 2006-01-20 12:13 UTC by Jurgen Kramer
Modified: 2007-11-30 22:11 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-01-22 17:11:30 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Partial screenshot of offending text (18.64 KB, image/jpeg)
2006-01-20 12:13 UTC, Jurgen Kramer
no flags Details
fix (1.08 KB, patch)
2006-01-20 17:41 UTC, Christopher Aillon
no flags Details | Diff

Description Jurgen Kramer 2006-01-20 12:13:44 UTC
Created attachment 123479 [details]
Partial screenshot of offending text

Comment 1 Jurgen Kramer 2006-01-20 12:13:44 UTC
Description of problem:
When the low battery message appears it shows HTML codes

Version-Release number of selected component (if applicable):
gnome-power-0.3.3-0.cvs.20060106

How reproducible:
Always

Steps to Reproduce:
1. Run laptop on batter until low batt message appears
2. Message shows <b>....</b> HTML codes instead of bold print
3.
  
Actual results:
Message shows ..approximately <b>15 minutes</b> of remaining...
The '15 minutes' text should probably be bold.

Expected results:
The '15 minutes' text should probably be bold not showing HTML tags.

Additional info:
Attachted partial screen shot.

Comment 2 Jurgen Kramer 2006-01-20 14:43:04 UTC
current version of gnome-power 0.3.4 still shows the HTML tags.


Comment 3 Richard Hughes 2006-01-20 16:05:11 UTC
The old version of libnotify allowed this. Is this a libnotify bug?
I'll cc j5.

Richard.

Comment 4 John (J5) Palmieri 2006-01-20 16:20:09 UTC
Yes, this is a known bug easy to fix.  Reasassigning to Chris.  The fix is easy.
 Ygu need to set the pango attribute to parse the markup but I belive you need
to also escape the string first (there is a function for that).  Just look at
the docs for PangoMarkup and even the gtklabel docs. 

Comment 5 Richard Hughes 2006-01-20 16:41:22 UTC
Known bug with g-p-m or libnotify? Thanks.

Comment 6 Christopher Aillon 2006-01-20 17:15:17 UTC
probably gpm since NM does the same but it works.  I'll look.

Comment 7 Christopher Aillon 2006-01-20 17:32:29 UTC
actually, nevermind.  i guess my NM patch breaks too.  looks like notify-daemon.

Comment 8 Christopher Aillon 2006-01-20 17:41:23 UTC
Created attachment 123494 [details]
fix

So, the problem is that we do escaping... J5, you mention we should do escaping
above as well.	I'm a bit confused.  escaping will turn <b> into &lt;b&gt;

Haven't yet tested, but I bet this patch fixes it -- is there any reason you
think we should be escaping this stuff, John?

Comment 9 John (J5) Palmieri 2006-01-20 17:46:33 UTC
No, by default all text renders do not use GMarkup to parse the string.  You
must turn that on.  As for escaping here is the issue:

The markup passed to gtk_label_set_markup() must be valid; for example, literal
</>/& characters must be escaped as &lt;, &gt;, and &amp;. If you pass text
obtained from the user, file, or a network to gtk_label_set_markup(), you'll
want to escape it with g_markup_escape_text() or g_markup_printf_escaped().

But I guess we don't know what we get from the user so it will up to
applications to do that.

Comment 10 Christopher Aillon 2006-01-20 18:22:34 UTC
John, you're wrong I think. 
http://developer.gnome.org/doc/API/2.0/gtk/GtkLabel.html#gtk-label-set-markup
<<<
Parses str which is marked up with the Pango text markup language, setting the
label's text and attribute list based on the parse results. If the str is
external data, you may need to escape it with g_markup_escape_text() or
g_markup_printf_escaped();
>>>

Going further to g_markup_escape_text:
http://developer.gnome.org/doc/API/2.0/glib/glib-Simple-XML-Subset-Parser.html#g-markup-escape-text
<<<
Escapes text so that the markup parser will parse it verbatim. Less than,
greater than, ampersand, etc. are replaced with the corresponding entities. This
function would typically be used when writing out a file to be parsed with the
markup pars
>>>

If we replace the entities, things don't work properly.

Also, the example at
http://developer.gnome.org/doc/API/2.0/glib/glib-Simple-XML-Subset-Parser.html#g-markup-printf-escaped
<<<
const char *store = "Fortnum & Mason";
const char *item = "Tea";
char *output;
 
output = g_markup_printf_escaped ("<purchase>"
                                  "<store>%s</store>"
                                  "<item>%s</item>"
                                  "</purchase>",
                                  store, item);

>>>

suggests that we only want to escape when we WANT the text to be treated
literally.  Here, we WANT the text of the ampersand to be treated as a literal
ampersand, and not the start of an entity.

Getting valid markup is definitely a concern.  Perhaps libnotify should attempt
to do some validation before it passes it to notify-daemon, but we have to rely
at some point that we are getting valid markup.  If we escape, then there is no
application that can use markup with libnotify without unescaping at some point.
 AFAIK, there is no API to do that.  If apps pass invalid markup, then the app
will look silly and the bug will be properly filed on the app.

I guess we need to decide whether we want apps to pass markup or not.

Comment 11 Christopher Aillon 2006-01-20 18:26:17 UTC
One thing we could do is use g_markup_parse_context_new() to create our own
parser, and make sure the output is valid before we send it on to
gtk_label_set_markup ()

Comment 12 Christopher Aillon 2006-01-22 17:11:30 UTC
this should be fixed in rawhide, now.


Note You need to log in before you can comment on or make changes to this bug.