Make WordPress Core

Opened 15 years ago

Closed 15 years ago

#12564 closed enhancement (wontfix)

comment_form() needs an output filter

Reported by: joostdevalk's profile joostdevalk Owned by: joostdevalk's profile joostdevalk
Milestone: Priority: normal
Severity: normal Version:
Component: Comments Keywords: has-patch dev-feedback
Focuses: Cc:

Description

The submit button in a comment form needs a filter hook to allow adding an onclick handler to it, for for instance tracking plugins or to allow a javascript alert or prompt.

Attachments (4)

comment-form-submit.diff (1006 bytes) - added by joostdevalk 15 years ago.
Patch
comment-form-submit.2.diff (1006 bytes) - added by joostdevalk 15 years ago.
Patch
test-comment.php (574 bytes) - added by joostdevalk 15 years ago.
Plugin to test last patch.
comment_form_filter.diff (871 bytes) - added by joostdevalk 15 years ago.
Patch

Download all attachments as: .zip

Change History (16)

#1 @joostdevalk
15 years ago

Discussion on IRC with nacin made me agree with him that there's a better solution to all this: a filter on the entire output of the comment_form function. Attaching patch that does this through an output buffer. Attaching new patch.

#2 @joostdevalk
15 years ago

  • Summary changed from Comment Submit buttons need an action hook to comment_form() needs an output filter

@joostdevalk
15 years ago

Plugin to test last patch.

#3 @joostdevalk
15 years ago

After more discussion with nacin on IRC I added an echo parameter, patch coming up.

#4 @nacin
15 years ago

  • Milestone changed from Unassigned to 3.0

#5 @nacin
15 years ago

Anyone have any objections to this?

#6 @nacin
15 years ago

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

(In [14268]) Add full HTML filter to comment_form(), along with an echo arg. props joostdevalk, fixes #12564.

#7 @westi
15 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I'm not a big fan of output buffering in general and would rather we avoid it unless there is no other solution.

Do we really need an output buffer here?

The use case of this ticket can all be handled with either a specific 'attribute' filter or just use JavaScript progressive enhancement to add it.

I would rather revert this change.

#8 @nacin
15 years ago

I'm not a fan of output buffering either, but then any plugin hooking into the various action hooks will need to be cognizant of returning data, instead of echoing it for display, which is more intuitive.

Honestly, I think the use case in this ticket calls for wontfix. But I didn't see a reason why we couldn't offer a generic output filter (and option to return a form) anyway.

#9 @westi
15 years ago

Ok.

I'm strongly against plugins scanning through chunks of output buffered content to add/remove stuff.

It is much more efficient and reliable to do this with javascript.

#10 @solarissmoke
15 years ago

+1 to westi's point of view. Also -- what happens if more than one plugin wants to attach an event to an onclick - without overwriting others?

#11 @westi
15 years ago

(In [14274]) Revert [14268] - It is much better to use JavaScript for this kind of enhancement. Output buffering is too fragile. See #12564.

#12 @westi
15 years ago

  • Milestone 3.0 deleted
  • Resolution set to wontfix
  • Status changed from reopened to closed

Closing as WONTFIX

Note: See TracTickets for help on using tickets.