[wp-hackers] Getting Bugs Noticed

[Jacob Santos]

That is actually a pretty good solution. I'll fix your patch (must be 
.diff or .patch and not .txt) and resubmit it.

I meant "good catch."

Jacob Santos

Brian Krausz wrote:
> Sorry for the slow reply:
>
> Since I'm the one to write the patch which contributed the most to
>   
>> _wp_filter_build_unique_id(), I'll say that you are doing it wrong. The
>> function was never meant to handle your use case. It is working as it
>> supposed to. The issue is that you are changing the data of what the
>> function sees as the same instance (it isn't, which is where it appears the
>> bug stems from).
>>     
>
>
> Exactly my point, two instances should not be considered the "same", since
> every instance of a class can have its own branching.  You can't assume a
> class will always add the same action as their first add_action call.
>
>
>   
>> If you work it out, the first instance works out to Foobaz0 and the second
>> instance should work out to Foobaz1. This is assuming that the plugin code
>> is isolated. I will have to take a further look at it, because it does not
>> seem like bar() is doing anything in your example, but the result should be
>> "12" and not just "2". So most likely your use case was not tested fully.
>> That is easy enough to correct, just write test cases for your example and
>> see what happens and if it fails, then attempt to fix it until the tests
>> pass.
>>     
>
>
> That's more or less what I did, but with my actual code: the exact fix was
> swapping the init call and the other call.
>
>
>   
>> Where you are incorrect is that once array(&$this, 'bar') is added to
>> 'something1', it should gain the _wp_filter_id to 0 and when array(&$this,
>> 'baz') is added to 'init' hook, the _wp_filter_id should already exist. You
>> are correct in that when the second instance is created and it goes to add
>> array(&$this, 'bar') to 'something2' it will not find _wp_filter_id, nor
>> will it find any other items in 'something2', so it will also set it to 0.
>> You are correct in that instead of getting the correct 'Foobaz0' and
>> 'Foobaz1', you end up with both 'Foobaz0'. Interestingly, if you had
>> something actually echoed in 'bar', you'll probably see both items (in
>> theory). In the case of the 'init' hook, the second instance is replacing
>> the first, which is why you only see '2'.
>>     
>
>
>   
>> This could be fixed if you had the 'init' hook first instead of second. It
>> is probably why the static method worked in your case, because for static
>> methods, the _wp_filter_id is not set, therefore when it gets to the init
>> hook, it sets it and then counts the amount of the hooks. It seems like your
>> bug is probably biting the asses of all of the dumbasses who are writing
>> their plugins incorrectly. Good chance!
>>     
>
>
> I understand the reasoning here, and know the solution to the situation, but
> IMHO the function doesn't follow its spec.  The spec (though not explicitly
> stated), is to hash an object so that it can be stored without risk of
> overwriting instances of other objects.  Since there exists a way to cause
> objects to overwrite each other while still following the spec of
> add_action, I see a problem.
>
>
>   
>> Unfortunately, I do not see a fix at this time. If you can think of another
>> algorithm to make the function more idiot proof, it should suffice until the
>> world creates a bigger idiot (not you, you found the problem and reported
>> it, I'm talking about those who are affected by this and don't know it).
>>     
>
>
> 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...
>
>
>   
>> Jacob Santos
>>
>> PS: It will it go a long way to getting things solved, if you create a
>> patch for a possible solution. I could really care less for #8731, therefore
>> someone else is going to need to look at it or you are going to have to
>> handle it yourself. Now, I might have some interest in it if you slap me
>> around a little and threaten to harm my next door neighbor who I could also
>> care less about.
>>     
>
>
> Thanks for the advice, I didn't realize patches push things along that
> well.  I'm away vacation soon, so my responses may be delayed, but my patch
> is attached to the bug.
>
> Thanks,
> Brian
>
> Brian Krausz wrote:
>   
>>>  Hey guys,
>>>
>>>  I have two bugs from a while back that seem to have gone ignored:
>>>
>>>  https://core.trac.wordpress.org/ticket/8723
>>>  https://core.trac.wordpress.org/ticket/8731
>>>
>>>  I'm wondering if there's something I'm doing that causes these bugs
>>>  to go unnoticed?  I feel bad assigning them to random people, but I
>>>  would like them to be discussed, and they need a consensus before I
>>>  can submit a patch for them.
>>>
>>>  Thanks, Brian _______________________________________________
>>>  wp-hackers mailing list wp-hackers at lists.automattic.com
>>>  http://lists.automattic.com/mailman/listinfo/wp-hackers
>>>
>>>       
>> _______________________________________________
>> wp-hackers mailing list
>> wp-hackers at lists.automattic.com
>> http://lists.automattic.com/mailman/listinfo/wp-hackers
>>
>>     
> _______________________________________________
> wp-hackers mailing list
> wp-hackers at lists.automattic.com
> http://lists.automattic.com/mailman/listinfo/wp-hackers
>