Opened 11 years ago

Closed 10 years ago

## #702 closed defect (fixed)

Reported by: Owned by: tag@… Alec Thomas normal AddCommentMacro critical 0.10

### Description

AddComment should actually be a plugin because the current mechanism of writing comments through post requests is both size limiting and means each time a user hits back, a new comment gets stuffed in the page.

Additionally, image attachements in the page may create duplicate comment entries. It'd be realice nice of someone would make this a plugin that didn't suffer from these problems.

### comment:1 Changed 11 years ago by Noah Kantrowitz

Priority: high → normal

This has nothing to do with macro/plugin, it is related to the fact that wiki macros (no matter how they come to be) cannot use POSTs, since the wiki's web UI captures those before macros are run. A number of possible solutions exist, but all are a bit hackish.

### comment:2 Changed 11 years ago by tag@…

Maybe a hack is the right thing to do until the trunk introduces a possibility. If you have some suggestions as to how it'd best be done, I'd like to look at doing it if now one else finds this is obnoxious as I do.

### comment:3 Changed 10 years ago by Jonathan S. Shapiro

Please forgive an ignorant question, because I'm only just starting to look at Trac internals, but would it work to make comments a new object type and have an [[AddComments]] macro in the "containing" page (or some such thing -- perhaps there is a better UI for this). How difficult is it to add new document types to Trac?

Let me clarify that I'ld be willing to actually work on this. We need the functionality, and I'm not happy with a couple of aspects of the comment mechanism, so please take this comment as a sanity check before I offer to actually help solve the problem -- I don't want to bite this off if it's an irrationally hard approach, or if other work that is going on seems likely to solve the problem soon.

### comment:4 Changed 10 years ago by Alec Thomas

The way this should be done is by adding an IRequestHandler for, say, /comment/<wiki-page> to handle the POSTs. An extra table for comments could be a possibility, though you could just update the Wiki page object itself as it is currently (probably better, as the comments are stored in the page history).

### comment:5 Changed 10 years ago by Jonathan S. Shapiro

Trac Release: 0.9 → 0.10

An unrelated bug seemed to imply that POST was the wrong way to go, and suggested conversion to a plugin for this reason. That bug is closed, but it is unclear to me whether the issue of a POST size limit was real or not.

One reason I suggest a new object type is that comments do not seem (to me) to be restricted to wiki pages. By handling them as separate object types they can be generalized to other things. This also seems (to me) more consistent with the emerging Trac Object Model work.

However, I say this with no understanding of what challenges it may entail.

### comment:6 Changed 10 years ago by tag@…

No.

GET Requests have the real size limit issue, they can only hold as much as a URL is capable less the actual space required for the URL (so < 1kb, under all circumstances).

POST requests however can accept up to whatever the server limit is which commonly is in the range of several megabytes, but is handled on a case-per-case basis.

Creating a new object type will be acceptable, but tricky. There is a correlation between each existing object (change set, wiki page, ticket, so forth) and creating the correlation of each of these individually will be a challenge. Also, keep in mind that some of the trac objects already have an existing comment mechanism (such as the one this discussion is taking place in now) and others have constraints that are likely to make comments less useful in them (such as change sets, possibly). The current add comment mechanism is while slightly archaic atleast in the same spirit as the way comments are handled for tickets (even if they are indeed pasted on top of the wiki system).

I'm really not very convinced there is a truely clean way to solve this problem as an afterthought, so until the next round of refactoring and/or consolidation takes place for track 0.11, we might want to just solve it as per the first approach.

### comment:7 Changed 10 years ago by Odd Simon Simonsen

Resolution: → fixed new → closed

The 0.11 version now solves this by using req.redirect() after a successful submit - like Trac does for all it's events that modifying data. Seems to work well.

I doubt I'll be updating it for older versions, so I'm closing this ticket as the original issue is fixed in latest version.

(Regarding a general commenting plugin: Nice idea, but I wouldn't start with this macro as a starting point... - save the idea for a new plugin :-)

thanks man

### Modify Ticket

Change Properties