301156 - HTML encoding fixes

Status: RESOLVED WORKSFORME

(Bugzilla :: Bugzilla-General, defect)

Description

[Matt Selsky [:selsky]]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.8) Gecko/20050524 Fedora/1.0.4-4 Firefox/1.0.4
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.8) Gecko/20050524 Fedora/1.0.4-4 Firefox/1.0.4

There are a bunch of places where '&' needs to be converted to '&', etc. 
The HTML Validator plugin for FireFox is very helpful in finding these problems.

Reproducible: Always

Comment 1

[Matt Selsky [:selsky]]

Attachment: Fixes encoding.

Comment 2

[Max Kanat-Alexander]

You may wish to look at http://www.bugzilla.org/reviewer-list.html to see who to
ask for review. If you ask for review "from the wind" (as it is now) it might
never actually get done.

Comment 3

[Max Kanat-Alexander]

Oops, that URL should have been:

http://www.bugzilla.org/docs/reviewer-list.html

I am definitely not the correct reviewer. :-)

Comment 4

[Vlad Dascalu]

Comment on attachment 189644 [details] [diff] [review]
Fixes encoding.

Per http://template-toolkit.org/docs/default/Manual/Filters.html#html , the
"FILTER html" thing in Template Toolkit "converts the characters '<', '>' and
'&' to '&lt;', '&gt;' and '&amp;', respectively".

In:

--- template/en/default/admin/groups/edit.html.tmpl	18 Feb 2005 16:38:42
-0000	   1.5
+++ template/en/default/admin/groups/edit.html.tmpl	18 Jul 2005 02:29:00
-0000

you have:

-	   <a href="[% "editgroups.cgi?action=changeform&group=${group.grpid}"
FILTER html %]">
+	   <a href="[%
"editgroups.cgi?action=changeform&amp;group=${group.grpid}" FILTER html %]">

which passes "&amp;" to the html filter. That would transform it into
"&amp;amp;", which is wrong.

I'd be happy to review the second version, in which the case above and all the
others (if any remaining) are fixed. Please request review from the wind if
that is the case.

Comment 5

[Vlad Dascalu]

mkanat, please stop modifying valid reviews from the wind. I've been involved in
open projects that have hundreds of "review" requests daily, and they use review
from the wind and are doing great. Both specific review and review from the wind
take statistically the same amount of time. If you can't guarantee a review
yourself, then don't assign it to a specific person. In this case, a number of
people would have been able to review the patch well before myk's.

Thank you.

Comment 6

[Max Kanat-Alexander]

I didn't modify it, the patch author did. :-) You are free to take any review
you feel qualified to give, as you know.

Comment 7

[Vlad Dascalu]

He did per your suggestion.

My reviews were not the topic. Again you're changing the topic. Oh well, I'll
stop trying.

Comment 8

[Dave Miller [:justdave]]

(In reply to comment #5)
> mkanat, please stop modifying valid reviews from the wind. I've been involved in
> open projects that have hundreds of "review" requests daily, and they use review
> from the wind and are doing great.

They might well be, but we know from experience that *we* don't (unless someone
has very recently started actively taking on unassigned reviews, but if that's
the case, I can't fault anyone for not being used to it yet).  But this isn't
the place for this.  Mail mkanat (or me) privately if you have a problem with
the way he's doing things.  Bring it up on the developers list if you don't get
answers you like from him or myself.  Thanks!

Comment 9

[victory <[e-mail]>]

Matt Selsky:
Only checked files the patch mentioned.
Below are still not fixed. are there other places to fix?
Note that escaping vars like title and atom that are passed to global/header.html.tmpl causes double escape ;-)
To avoid this, you'll have to check how the vars are used ;-)
template/en/default/admin/flag-type/list.html.tmpl     line 116
template/en/default/admin/groups/delete.html.tmpl      line 60, 91
template/en/default/attachment/edit.html.tmpl          line 107,275,280
template/en/default/bug/create/create-guided.html.tmpl line 125,126
template/en/default/reports/create-chart.html.tmpl     line 192

Comment 10

[Frédéric Buclin]

I think most places have been fixed in various bugs. If there are still some places broken, feel free to reopen this bug and enumerate places which need to be fixed. Marking as WFM for now.