Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Got this error in the latest dev.
In flag.module it is:
function flag_flag_access_multiple($flag, $content_ids, $account)
And in flag.inc:
function type_access_multiple($content_ids, $account = NULL)
Comment | File | Size | Author |
---|---|---|---|
#15 | flag-fix_method_declaration-1475040-15.patch | 810 bytes | drupalmonkey |
#12 | 1475040.12.typeMultipleAccess.patch | 495 bytes | socketwench |
#2 | type_access_multiple-1475040-2.patch | 1.82 KB | MiSc |
#1 | type_access_multiple-1475040-1.patch | 1.55 KB | MiSc |
Comments
Comment #1
MiSc CreditAttribution: MiSc commentedIn the attached patch, I changed it to:
function type_access_multiple($flag, $content_ids, $account)
Comment #2
MiSc CreditAttribution: MiSc commentedSeems that the $flag variable is missing in access_multiple, added global flag to that function. Should be considered a temp fix I think...
Comment #3
apotek CreditAttribution: apotek commentedI applied this patch cleanly to flag.inc, and my notices went away. this looks good to me, though, as MiSc notes, having to declare $flag as global is less than ideal, but I think that would be work best left to another issue?
Comment #4
apotek CreditAttribution: apotek commentedI'm thinking about this issue about the global $flag, and it looks more and more to me like this code
The more I look at it, it seems there is no way to resolve this issue with the $global. flag_flag_access_multiple() actually uses the $flag that is passed (although it's evidently broken in current dev since whatever would be passed from flag_* classes would be missing the flag param (until this patch is applied).
The alternative to MiSc's patch would be to change this in flag.module
to this:
Looks like some thought might need to be put into how global $flag is intended to be used across this module. Until that can be done, I think this patch is the best step forward for the moment. I think it's ready to commit.
Comment #5
caw67 CreditAttribution: caw67 commentedpatch #2 works for me http://drupal.org/node/1475040#comment-5709920
Comment #6
joachim CreditAttribution: joachim commentedNeither of these approaches are correct, I'm afraid!
Let's review what we have:
- flag_flag_access_multiple($flag, $content_ids, $account) is a hook implementation.
- type_access_multiple($content_ids, $account) is a method on a flag class. It doesn't need the $flag -- because it *is* the current flag. Nor does it need the global $flag, for the same reason. The current flag is $this.
So these two functions don't have anything to do with each other in terms of the PHP's perception of them and both of these definitions are fine.
The problem is that the different implementations of the method don't match, as grep shows:
The second one puts the second parameter as optional -- that's what PHP complains about, I imagine (though I'm not getting that error but it could be because my error level is different).
Comment #7
apotek CreditAttribution: apotek commentedThank you. You might be on the right track, but you're working off a later codebase. None of my type_access_multiple() signatures include a NULL as a default $account value.
Maybe this ticket is out-of-date with latest devs?
Comment #8
joachim CreditAttribution: joachim commentedCan you try with the latest dev please?
I'd say that this is wrong and needs fixing anyway:
> 1324: function type_access_multiple($content_ids, $account = NULL) {
Comment #9
joachim CreditAttribution: joachim commentedComment #10
Marlon CreditAttribution: Marlon commentedSorry, still getting this error.
Comment #11
joachim CreditAttribution: joachim commentedYes, that's not surprising. Someone needs to take the time to write a patch to fix this.
Comment #12
socketwench CreditAttribution: socketwench commentedIf I understood the thread correctly, the signature of type_multiple_access() needs to be synced up across implementations. The only one I found that was different was in flag_node:type_multiple_access().
Comment #13
joachim CreditAttribution: joachim commentedThanks for the patch!
(Dunno where all the novices are though! :/)
Comment #14
joachim CreditAttribution: joachim commentedComment #15
drupalmonkey CreditAttribution: drupalmonkey commentedPatched against 7.x-2.x.
Comment #16
joachim CreditAttribution: joachim commentedThanks!
Applies to both 7x2x and 6x2x.