Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.

Bug 178420

Summary: Low battery message shows HTML codes
Product: [Fedora] Fedora Reporter: Jurgen Kramer <gtmkramer>
Component: notify-daemonAssignee: Christopher Aillon <caillon>
Status: CLOSED RAWHIDE QA Contact:
Severity: medium Docs Contact:
Priority: medium    
Version: 5CC: johnp, richard
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2006-01-22 17:11:30 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Bug Depends On:    
Bug Blocks: 150221    
Attachments:
Description Flags
Partial screenshot of offending text
none
fix none

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.