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

[Ryan J Ollos]

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

@@ -95 +95 @@
 
         buffer = StreamBuffer()
         return stream | Transformer('//div[@class="description"]'
-                                    '/h3[@id="comment:description"]') \
+                                    '/*[@id="comment:description"]') \
             .after(delete_ticket).end() \
             .select('//div[starts-with(@class, "change")]/@id') \
             .copy(buffer).end() \

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

[Christian Boos]

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…

[Ryan J Ollos]

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.

[Christian Boos]

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.

[Ryan J Ollos]

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.

[Ryan J Ollos]

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.