Edgewall Software
Modify

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#11100 closed enhancement (wontfix)

tracopt.ticket.deleter Genshi stream transformer may not work with themes

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone:
Component: ticket system Version: 1.0-stable
Severity: normal Keywords:
Cc: Branch:
Release Notes:
API Changes:

Description

The XPath expression relies on an h3 being present, but a theme may want to change this to, for example, an h5. Would there be any harm in the following change, since the id elements must be unique anyway:

  • tracopt/ticket/deleter.py

    diff --git a/tracopt/ticket/deleter.py b/tracopt/ticket/deleter.py
    index 75b93fa..75b1724 100644
    a b class TicketDeleter(Component):  
    9595
    9696        buffer = StreamBuffer()
    9797        return stream | Transformer('//div[@class="description"]'
    98                                     '/h3[@id="comment:description"]') \
     98                                    '/*[@id="comment:description"]') \
    9999            .after(delete_ticket).end() \
    100100            .select('//div[starts-with(@class, "change")]/@id') \
    101101            .copy(buffer).end() \

This would also seem to be more robust against future changes to the Trac ticket template.

Attachments (0)

Change History (7)

comment:1 by Christian Boos, 6 years ago

Milestone: 1.0.2

This makes me wonder about the speed implications of this. In Genshi, instead of comparing the ids for h3 elements, we'll compare them for every element. Well, it'll go from slow to slower, so we probably won't notice much.

Still, for how such things should ideally be done IMO, see #10735. In Javascript at least, going to an #id is as immediate as it can be…

in reply to:  1 comment:2 by Ryan J Ollos, 6 years ago

Replying to cboos:

Still, for how such things should ideally be done IMO, see #10735. In Javascript at least, going to an #id is as immediate as it can be…

Should we make this ticket then about implementing the "add button" functionality in JavaScript? I'll plan to workup a patch for that.

comment:3 by Christian Boos, 6 years ago

The "delete button" you mean? I think 1.1.x would be a good target for this. If we want to fix the present defect for 1.0.2, the proposed patch looks good enough.

in reply to:  3 comment:4 by Ryan J Ollos, 6 years ago

Replying to cboos:

The "delete button" you mean?

Hah, yes it was a poorly phrased attempt to say "code that adds the delete button".

I think 1.1.x would be a good target for this. If we want to fix the present defect for 1.0.2, the proposed patch looks good enough.

Okay, sounds good. I'll plan to do the 1.1.x-targeted change in another ticket then.

comment:5 by Ryan J Ollos, 6 years ago

Owner: set to Ryan J Ollos
Reporter: changed from Ryan J Ollos <ryan.j.ollos@…> to Ryan J Ollos
Status: newassigned

comment:6 by Ryan J Ollos, 6 years ago

Resolution: wontfix
Status: assignedclosed

Given the potential performance penalty, that this change alone wouldn't resolve bh:#427 and isn't general enough to resolve all theme-related issues (as replacing the ITemplateStreamFilter implementation with JavaScript could do), I'm going to just close this as wontfix. Note also that proposed changes in #10948 reposition the modify / clone / delete buttons.

Last edited 6 years ago by Ryan J Ollos (previous) (diff)

comment:7 by anonymous, 6 years ago

Milestone: 1.0.2

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.