Opened 2 years ago

Closed 2 years ago

Last modified 2 years 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 2 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 2 years ago by DefaultCC Plugin

  • Cc jknight added

comment:2 Changed 2 years ago by borko

  • Author set to borko
  • Cc borko84@… added

Changed 2 years ago by borko

comment:3 Changed 2 years ago by borko

  • Keywords review added

comment:4 Changed 2 years 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 2 years 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 2 years ago by glyph

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:7 Changed 2 years ago by glyph

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

comment:8 Changed 2 years ago by borko

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

comment:9 Changed 2 years 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.