#8723 closed defect (bug) (fixed)
Instances of same class that call variable hook first generate same wp_filter_id
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 2.8 | Priority: | normal |
Severity: | major | Version: | 2.7 |
Component: | Plugins | Keywords: | trac-attention |
Focuses: | Cc: |
Description
Best shown by example:
<?php class Foo { var $i; function Foo($i) { $this->i = $i; add_action('something'.$i, array(&$this, 'bar')); add_action('init', array(&$this, 'baz')); } function bar() {} function baz() { echo $this->i; } } new Foo(1); new Foo(2); ?>
This results in only seeing "2". This is because _wp_filter_build_unique_id sets $function[0]->wp_filter_id to 0 for both. The following line is the issue:
$count = isset($wp_filter[$tag][$priority]) ? count((array)$wp_filter[$tag][$priority]) : 0;
Since add_action is being called for the instance for a unique $tag in its first instance, $wp_filter[$tag][$priority] is empty for both instances...there should be some way to prevent this.
This may seem like a rare case, but I had to hunt this down for my plugin when the first action was:
add_action('wp_ajax_add-' . $this->slug, array(&$this, 'ajax_add'));
The fix is trivial: call add_action on something with a static name first, but is difficult to discover.
Attachments (3)
Change History (18)
#4
@
16 years ago
Explanation, from wp-hackers:
What about using a static var that's incremented with each read to fill wp_filter_id? That guarantees it will be unique amongst other objects. It could track class names so the id stays smaller, but I see no benefit to that...
#7
@
16 years ago
- Keywords has-patch tested added; needs-patch needs-testing removed
There are two ways to handle it. You can prevent that your class becomes the same wp_filter_id over and over again by specifying it yourself before registering any hooks.
/* constructor of class Foo */ function Foo($i) { $this->i = $i; $this->wp_filter_id = 'Foobar' . $i; ...
the patch has been fixed. The patch ensures that objects who do not have such a default wp_filter_id set get a unique value attached now (that is the intended functionality) even for the same class.
additionally i corrected the docblocks a bit to clarify parameter types and return type description.
#10
follow-up:
↓ 13
@
16 years ago
- Keywords commit removed
- Milestone changed from 2.8 to 2.8.1
- Resolution fixed deleted
- Status changed from closed to reopened
- Version changed from 2.7 to 2.8
unfortunately, this patch has a major problem with objects that don't already have wp_filter_id set. One value is stored as wp_filter_id on the object($filter_id_count++
), but an entirely different value is appended to the index that gets returned (isset($wp_filter[$tag][$priority]) ? count((array)$wp_filter[$tag][$priority]) : 0;
). Therefore, it's completely impossible to remove a filter on an object that was added in this way.
It seems like everyone is okay with the idea of using a static counter variable. If that's the case, then there is no need to mess around with the array size of $wp_filter[$tag][$priority]
... just use the counter. And if $priority isn't being used in this manner, then it is no longer needed. This actually greatly simplifies this method, as the following patch shows.
#11
@
16 years ago
@wpnorris: you sure this works with static classes?
class foo { function bar() { } } add_action('hook', array('foo', 'bar'));
I might be wrong, but I believe foo->wp_filter_id++ is fatal in some php versions.
#12
@
16 years ago
- Keywords needs-testing added; tested removed
mm, nevermind that. I suppose it'll work fine since there already is a line like that in the existing code.
#13
in reply to:
↑ 10
@
16 years ago
- Keywords has-patch needs-testing removed
- Resolution set to fixed
- Status changed from reopened to closed
- Version changed from 2.8 to 2.7
Replying to wnorris:
I think you're mixing two things here. The one thing is that you use(d) a (unstable) defintion of array keys in your code. That's the one side and that is a bug in your code IMHO.
The other side is, that this tickets fix was for a function called _wp_filter_build_unique_id which job it is to return a unique id per filter. The only aim of that function is to provide a unique id per filter per request.
That function had a bug. That bug was fixed.
The problem you face might be related to the bugfix but should not go into this ticket. Feel free to open a new ticket, reference this one and write, that a bug has been introduced by that certain commit in your opinion. You can then reference the new ticket here as well.
Proposed patch