WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#12564 closed enhancement (wontfix)

comment_form() needs an output filter

Reported by: joostdevalk Owned by: 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 8 years ago.
Patch
comment-form-submit.2.diff (1006 bytes) - added by joostdevalk 8 years ago.
Patch
test-comment.php (574 bytes) - added by joostdevalk 8 years ago.
Plugin to test last patch.
comment_form_filter.diff (871 bytes) - added by joostdevalk 8 years ago.
Patch

Download all attachments as: .zip

Change History (16)

#1 @joostdevalk
8 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
8 years ago

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

@joostdevalk
8 years ago

Plugin to test last patch.

#3 @joostdevalk
8 years ago

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

#4 @nacin
8 years ago

  • Milestone changed from Unassigned to 3.0

#5 @nacin
8 years ago

Anyone have any objections to this?

#6 @nacin
8 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
8 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
8 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
8 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
8 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
8 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
8 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.