Opened 21 months ago

Closed 21 months ago

Last modified 21 months ago

#6238 enhancement closed wontfix (wontfix)

t.w._flatten._flattenElement should support t.p.urlpath.URLPath

Reported by: tom.prince Owned by:
Priority: normal Milestone:
Component: web Keywords: web templates
Cc: jknight, borko84@… Branch:
Author: borko Launchpad Bug:

Description

I think the right thing to do here is to turn it into a string.

Attachments (1)

6238_v1.diff (2.7 KB) - added by borko 21 months ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 21 months ago by DefaultCC Plugin

  • Cc jknight added

comment:2 Changed 21 months ago by borko

  • Author set to borko
  • Cc borko84@… added

Changed 21 months ago by borko

comment:3 Changed 21 months ago by borko

  • Keywords review added

comment:4 Changed 21 months ago by exarkun

  • Keywords review removed

Hi borko. Thanks for working on this.

Sticking to the high-level, it's fairly important that the URL be properly escaped in the output of flatten. This patch so far makes no attempt to apply the proper escaping. Proper escaping here is made more difficult, I think, by the fact that URLPath doesn't bother to document what the result of using str() on it will be, nor whether it its various attributes store encoded or decoded data.

I think perhaps it's a bad idea to try to write any code that uses URLPath, due to the shortcomings of URLPath. A better course would be to work on improving URLPath (or replacing it) to the point where it's possible to be confident in uses of it (in other words, I don't think it is possible to have a satisfactory resolution to this ticket at this time).

At the very minimum, the tests for this feature need to include coverage for the various special characters a URL can include, and the two different flattener cases - inAttribute = True and inAttribute = False.

comment:5 Changed 21 months ago by borko

  • Resolution set to fixed
  • Status changed from new to closed

Ok. So I opt for creating new ticket for changing URLPath in a way it should be done with precise comment, setting this ticket as blocked and writing short note about this fact including a link to the ticket that must be done before. Thx.

comment:6 Changed 21 months ago by glyph

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:7 Changed 21 months ago by glyph

  • Resolution set to wontfix
  • Status changed from reopened to closed

comment:8 Changed 21 months ago by borko

so what's the final status of this ticket? are we closing it without any changes to URLPath ?

comment:9 Changed 21 months ago by glyph

Just to be clear about my changes, I was simply (dumbly) reacting to the fact that this was closed as "fixed". If it hasn't actually been fixed but it's just waiting to be reopened, 'wontfix' is a more appropriate status. I haven't had time to review it to see if further action is warranted :).

Note: See TracTickets for help on using tickets.