398701 - Replace |FILTER url_quote| by |FILTER uri|

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Frédéric Buclin]

url_quote() has been introduced by terry in 1998 in Bugzilla 2.0, prior to the introduction of |FILTER uri| by TT in version 2.06 in 2001. Now that FILTER uri is supported internally by TT, and now that this filter correctly escapes all reserved characters, since TT 2.16, we should get rid of our home-made url_quote() routine/filter and use the TT internal one.

The filter in TT 2.15 and older didn't escape all characters correctly, so this also means we have to bump the min requirement for TT to 2.16. As a side-effect, this bump will also help bug 23473 and bug 345346, so marking them as blocked by the current bug as well.

Comment 1

[Frédéric Buclin]

Note that url_quote() is called directly from editproducts.cgi and editcomponents.cgi, as well as from some other .pm modules. So this bug is specific to the removal of this filter from Template.pm and templates, and not about the removal of this routine itself from Util.pm.

Comment 2

[Frédéric Buclin]

I just checked, and I noted that TT 2.16 and TT 2.17 are not available for download. They were only interim releases which have been merged into the publicly available TT 2.18. So we should require TT 2.18 instead. Note that TT 2.18 is only 2 weeks younger than TT 2.16 anyway, so this doesn't make a big difference.

One problem though: TT 2.18 has been releases on February 9th, 2007, which is pretty recent. Maybe too recent to require it in Bugzilla 3.2?

Comment 3

[Max Kanat-Alexander]

We could just wait until 4.0. It's not extremely urgent. For now we can reverse it, and use url_quote everywhere instead of uri. That'll be a smaller patch, at least.

Comment 4

[Frédéric Buclin]

(In reply to comment #3)
> We could just wait until 4.0.

OK, we can bump the min req for TT as soon as we branch.


> and use url_quote everywhere instead of uri.

Let's do that in bug 264785.

Comment 5

[Frédéric Buclin]

Due to bug 457524, we will require TT 2.22, so the min requirement of 2.18 will automatically be satisfied.

Comment 6

[Frédéric Buclin]

Attachment: patch, v1

I used:

 find . -type f -name "*.tmpl" -exec replace 'FILTER url_quote' 'FILTER uri' -- {} \;

for the conversion in templates.

Comment 7

[Max Kanat-Alexander]

Comment on attachment 459417 [details] [diff] [review]
patch, v1

Cool. Looks good! :-)

We should also have our internal url_quote call Template::Filters::uri_filter, I think, so that we don't have to maintain our own code there anymore, and so that we have totally consistent uri-quoting in Bugzilla. We could also just use URI::Escape (which is what uri_filter is copied from), because we require URI. However, that would all be something to do in another bug.

Comment 8

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/             
modified Bugzilla/Template.pm                                                            
modified docs/en/xml/customization.xml                                                   
modified t/008filter.t                                                                   
modified template/en/default/config.rdf.tmpl                                             
modified template/en/default/sidebar.xul.tmpl                                            
modified template/en/default/account/profile-activity.html.tmpl                          
modified template/en/default/account/email/change-new.txt.tmpl                           
modified template/en/default/account/email/change-old.txt.tmpl                           
modified template/en/default/account/email/request-new.txt.tmpl                          
modified template/en/default/account/password/forgotten-password.txt.tmpl                
modified template/en/default/account/prefs/saved-searches.html.tmpl                      
modified template/en/default/admin/table.html.tmpl                                       
modified template/en/default/admin/classifications/edit.html.tmpl                        
modified template/en/default/admin/classifications/select.html.tmpl                      
modified template/en/default/admin/components/confirm-delete.html.tmpl                   
modified template/en/default/admin/components/edit.html.tmpl                             
modified template/en/default/admin/components/footer.html.tmpl                           
modified template/en/default/admin/components/list.html.tmpl                             
modified template/en/default/admin/custom_fields/edit.html.tmpl                          
modified template/en/default/admin/fieldvalues/confirm-delete.html.tmpl                  
modified template/en/default/admin/fieldvalues/footer.html.tmpl                          
modified template/en/default/admin/fieldvalues/list.html.tmpl                            
modified template/en/default/admin/groups/delete.html.tmpl                               
modified template/en/default/admin/keywords/edit.html.tmpl                               
modified template/en/default/admin/milestones/confirm-delete.html.tmpl                   
modified template/en/default/admin/milestones/footer.html.tmpl                           
modified template/en/default/admin/milestones/list.html.tmpl                             
modified template/en/default/admin/params/editparams.html.tmpl                           
modified template/en/default/admin/params/index.html.tmpl                                
modified template/en/default/admin/products/confirm-delete.html.tmpl                     
modified template/en/default/admin/products/edit.html.tmpl                               
modified template/en/default/admin/products/footer.html.tmpl                             
modified template/en/default/admin/products/list.html.tmpl                               
modified template/en/default/admin/products/updated.html.tmpl                            
modified template/en/default/admin/sanitycheck/messages.html.tmpl
modified template/en/default/admin/users/confirm-delete.html.tmpl
modified template/en/default/admin/users/listselectvars.html.tmpl
modified template/en/default/admin/users/responsibilities.html.tmpl
modified template/en/default/admin/users/userdata.html.tmpl
modified template/en/default/admin/versions/confirm-delete.html.tmpl
modified template/en/default/admin/versions/footer.html.tmpl
modified template/en/default/admin/versions/list.html.tmpl
modified template/en/default/attachment/cancel-create-dupe.html.tmpl
modified template/en/default/attachment/diff-header.html.tmpl
modified template/en/default/bug/dependency-tree.html.tmpl
modified template/en/default/bug/field-label.html.tmpl
modified template/en/default/bug/navigate.html.tmpl
modified template/en/default/bug/show-multiple.html.tmpl
modified template/en/default/bug/create/create-guided.html.tmpl
modified template/en/default/bug/create/create.html.tmpl
modified template/en/default/global/choose-classification.html.tmpl
modified template/en/default/global/choose-product.html.tmpl
modified template/en/default/global/common-links.html.tmpl
modified template/en/default/global/messages.html.tmpl
modified template/en/default/global/site-navigation.html.tmpl
modified template/en/default/global/useful-links.html.tmpl
modified template/en/default/global/user-error.html.tmpl
modified template/en/default/list/list.html.tmpl
modified template/en/default/list/list.ics.tmpl
modified template/en/default/list/quips.html.tmpl
modified template/en/default/list/table.html.tmpl
modified template/en/default/reports/components.html.tmpl
modified template/en/default/reports/create-chart.html.tmpl
modified template/en/default/reports/delete-series.html.tmpl
modified template/en/default/reports/duplicates-table.html.tmpl
modified template/en/default/reports/edit-series.html.tmpl
modified template/en/default/reports/keywords.html.tmpl
modified template/en/default/reports/report-table.html.tmpl
modified template/en/default/reports/report.html.tmpl
Committed revision 7394.

Comment 9

[Frédéric Buclin]

I missed the ones being in extensions/Voting/. Reopening!

Comment 10

[Frédéric Buclin]

Attachment: patch for extensions/Voting, v1

Comment 11

[Max Kanat-Alexander]

Comment on attachment 469978 [details] [diff] [review]
patch for extensions/Voting, v1

Obviously correct. :-)

Comment 12

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified extensions/Voting/template/en/default/hook/admin/products/updated-changes.html.tmpl
modified extensions/Voting/template/en/default/hook/bug/edit-after_importance.html.tmpl
modified extensions/Voting/template/en/default/pages/voting/bug.html.tmpl
modified extensions/Voting/template/en/default/pages/voting/user.html.tmpl
Committed revision 7447.