Roundup Tracker - Issues

Issue 2550795

classification
@dispname query args in page.html search links not valid html
Type: behavior Severity: normal
Components: Web interface Versions: 1.4
process
Status: fixed fixed
:
: rouilj : pcaulagi, rouilj, schlatterbeck
Priority: low : patch

Created on 2013-02-23 04:46 by rouilj, last changed 2016-07-22 20:43 by rouilj.

Files
File name Uploaded Description Edit Remove
url_encode_dispname.diff rouilj, 2016-07-18 01:41
Messages
msg4801 Author: [hidden] (rouilj) Date: 2013-02-23 04:46
In the classic (and minimal, devel and responsive) templates
the page.html menu column has a number of href links that include
entries like:

   '@dispname': i18n.gettext('Show Unassigned')

the space needs to be url encoded to %20 so that the page will validate.
These can be manually changed to %20, so they read:

   '@dispname': i18n.gettext('Show%20Unassigned')

The user's defined queries at the top of the column needs to be
fixed as well. To fix those replace:

  a href="#" tal:attributes="href
string:${qs/klass}?${qs/url}&@dispname=${qs/name}"
       tal:content="qs/name"

with:

  a href="#" tal:attributes="href
python:'%s?%s&@dispname=%s'%(qs.klass,qs.url,utils.urlquote(qs.name.plain()))"
       tal:content="qs/name"

and add a file with the following contents in the extension
directory of the tracker:

==============
import urllib

def urlquote(item):
    ''' make urllib.quote callable from a template.
    '''
    return urllib.quote(item)

def init(instance):
    instance.registerUtil('urlquote', urlquote)
===============

to make the utils.urlquote function exist. I am not sure if
there is a way to access urllib.quote directly without this utility
function or not.
msg4803 Author: [hidden] (pcaulagi) Date: 2013-02-24 05:05
> In the classic (and minimal, devel and responsive) templates
> the page.html menu column has a number of href links that include
> entries like:
>
>     '@dispname': i18n.gettext('Show Unassigned')
>
> the space needs to be url encoded to %20 so that the page will validate.
> These can be manually changed to %20, so they read:
>
>     '@dispname': i18n.gettext('Show%20Unassigned')

How can I test the translations?  I have not seen a tracker in a 
language other than English.  So how should I test that?

Normally, I have seen that companies get the translations done from an 
external agency.  So in this case, the additional characters would 
confuse the translators.
>
> The user's defined queries at the top of the column needs to be
> fixed as well. To fix those replace:
>
>    a href="#" tal:attributes="href
> string:${qs/klass}?${qs/url}&@dispname=${qs/name}"
>         tal:content="qs/name"
>
> with:
>
>    a href="#" tal:attributes="href
> python:'%s?%s&@dispname=%s'%(qs.klass,qs.url,utils.urlquote(qs.name.plain()))"
>         tal:content="qs/name"
>
Instead, can't we have qs.name.plain() always to urllib.quote before 
returning?
msg4804 Author: [hidden] (rouilj) Date: 2013-02-24 05:19
In message <51299F91.7040900@wwstay.com> <51299F91.7040900@wwstay.com>,
Pradip Caulagi writes:
>Pradip Caulagi added the comment:
>> In the classic (and minimal, devel and responsive) templates
>> the page.html menu column has a number of href links that include
>> entries like:
>>
>>     '@dispname': i18n.gettext('Show Unassigned')
>>
>> the space needs to be url encoded to %20 so that the page will validate.
>> These can be manually changed to %20, so they read:
>>
>>     '@dispname': i18n.gettext('Show%20Unassigned')
>
>How can I test the translations?  I have not seen a tracker in a 
>language other than English.  So how should I test that?

That's a valid question and one I wondered about as well. It is quite
possible that some languages like german would have a single word
(without any spaces) to describe 'Show Unassigned".

>Normally, I have seen that companies get the translations done from an 
>external agency.  So in this case, the additional characters would 
>confuse the translators.

Yeah the translator wouldn't recognize that "Show%20Unassigned"
requires translating just "Show Unassigned". I suppose that should be
replaced by:

 '@dispname': util.urlquote(i18n.gettext('Show Unassigned'))

hopefully the precompile will realise that the expression is static
and do all the work during the precompile and not at runtime, but it
may not be able to do that.

>> The user's defined queries at the top of the column needs to be
>> fixed as well. To fix those replace:
>>
>>    a href="#" tal:attributes="href
>> string:${qs/klass}?${qs/url}&@dispname=${qs/name}"
>>         tal:content="qs/name"
>> with:
>>
>>    a href="#" tal:attributes="href
>> python:'%s?%s&@dispname=%s'%(qs.klass,qs.url,utils.urlquote(qs.name.plain(
>)))"
>>         tal:content="qs/name"
>>
>Instead, can't we have qs.name.plain() always to urllib.quote before 
>returning?

Nope because we want the value with the space as the actual text of
the link. It just can't occur in the href. The full link code reads:

   <a href="#" tal:attributes="href python:'%s?%s&@dispname=%s'%(qs.klass,qs.url,utils.urlquote(qs.name.plain()))"
       tal:content="qs/name">link</a>

note the tal:content of qs/name which is qs.name.plain IIRC.
msg4813 Author: [hidden] (schlatterbeck) Date: 2013-02-26 17:32
On Sun, Feb 24, 2013 at 10:35:21AM +0530, Pradip P Caulagi wrote:
> 
> How can I test the translations?  I have not seen a tracker in a 
> language other than English.  So how should I test that?

You should add e.g. '&@language=de' to the get-request. Roundup will
store this in the session for subsequent requests, so once set the
language should be persistent. To switch back to english, use
'&@language=en'. I think this works even if you have disabled
'use_browser_language' in config.ini section [web].

Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   http://www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com
allmenda.com member                     email: rsc@allmenda.com
msg5867 Author: [hidden] (rouilj) Date: 2016-07-18 01:41
It looks like this issue was handled by switching to using:

    <a href="#"
       tal:attributes="href python:request.indexargs_url('issue', {
      '@sort': '-activity',
      '@group': 'priority',
      '@filter': 'status,assignedto',
      '@columns': columns,
      '@search_text': '',
      'status': status_notresolved,
      'assignedto': '-1',
      '@dispname': i18n.gettext('Show Unassigned'),
     })"

for the default queries. So it looks like indexargs_url is doing the
required %20 url encoding etc. If I use:

  utils.url_quote(i18n.gettext('Show Unassigned'))

@displname is Show%2520Unassigned and is displayed as Show%20Unassigned
as the page title. So it gets double encoded.

Also there is a utils.url_quote function in roundup/cgi/templating.py
so no need to add one to the extensions directory.

However the "Your queries" has the original issue. Spaces in the
query names sent as @dispname are not escaped. I added a url_quote function:

    def url_quote(self):
        """ Return the string in plain format but escaped for use in a
url """
        return urllib.quote(self.plain())

to templating.py:StringHTMLProperty and changed the query display code to:

    <tal:block tal:repeat="qs request/user/queries">
-    <a href="#" tal:attributes="href
string:${qs/klass}?${qs/url}&@dispname=${qs/name}"
+    <a href="#" tal:attributes="href
string:${qs/klass}?${qs/url}&@dispname=${qs/name/url_quote}"
        tal:content="qs/name">link</a><br>

(change qs/name to qs/name/url_quote for the @dispname parameter only).

Patch attached changing all the default templates. It doesn't
look like the jinja2 template displays the queries so it doesn't have
this issue.

Thoughts?

-- rouilj
msg5885 Author: [hidden] (rouilj) Date: 2016-07-22 20:43
No thought this week, so I checked it in. See changeset: 882fa4d9bead

I added tests for it. Also while I was there I added some more tests for
other StringHTMLProperty methods in: ae2a5d1afdd5.

Closing.
History
Date User Action Args
2016-07-22 20:43:53rouiljsetstatus: new -> fixed
resolution: fixed
messages: + msg5885
2016-07-18 01:41:42rouiljsetassignee: rouilj
2016-07-18 01:41:22rouiljsettitle: @dispname query args in page.html search links not valid html -> @dispname query args in page.html search links not valid html
2016-07-18 01:41:04rouiljsetkeywords: + patch, - Effort-Low
files: + url_encode_dispname.diff
messages: + msg5867
2013-02-26 17:32:46schlatterbecksetnosy: + schlatterbeck
messages: + msg4813
title: @dispname query args in page.html search links not valid html -> @dispname query args in page.html search links not valid html
2013-02-24 05:19:51rouiljsetmessages: + msg4804
title: @dispname query args in page.html search links not valid html -> @dispname query args in page.html search links not valid html
2013-02-24 05:05:29pcaulagisetnosy: + pcaulagi
messages: + msg4803
title: @dispname query args in page.html search links not valid html -> @dispname query args in page.html search links not valid html
2013-02-23 04:46:15rouiljcreate