[dokuwiki] Re: the event system

Chris Smith wrote:

Jason Grout wrote:

Chris Smith wrote:
@Jason. I don't think there is any need to add a "stop further advisements" facility. Can you give an example of where it could be useful? 

There are two reasons I added that:
1. To my understanding, that is what the stopPropagation() function already does. I was carrying forward (what I thought was) an already-existing feature. Why a BEFORE callback would prevent other BEFORE callbacks from being processed is debatable, though, especially if the order in which the callbacks are processed is not known.
Partly because it was carrying out three functions. - If it carries out a different version of the action any other BEFORE handlers are no longer BEFORE (this reason would be removed by a change to three advisements). - If it alters the event data in such a way as to make it unreliable for other handlers to process the data. 
- If it terminates the event.


Those reasons make sense.  I guess I should call stopPropagation in my
plugin right now, then.
iirc, stopPropagation only prevents further propagation of the current advisement, it doesn't have any effect on other advisements. 

Okay, that was what I was assuming (stopPropagation only stops the
current advisement), but apparently didn't state clearly.
2. stopPropagation() called in a primary callback (the new advisement) is something like the preventDefault(), just relabeled to be consistent with the idea that no other callbacks in the primary callback list will be called. 

Two comments:
1. the stopPropagation function doesn't seem to make much sense when there is no control over the order in which the callbacks are processed.
Order shouldn't affect whether or not its called. The choice to call stopPropagation should be based on what the handler does. A plugin can not know what other plugins are installed in the system. Because of that it can't make any assumptions about what it comes before or after. A plugin should make the decision to call stopPropagation because it has done something that makes any further handlers unsafe or unnecessary. 



Aaah, I think I understand better now.  Going back to an idea from last
April, there is a contract between DokuWiki and a plugin about what is
in the information passed with an event.  If a plugin violates that
contract by changing the information, then other plugins should not be
called.  Is that a correct way to think about it?  I can see a case made
for passing the event data by reference into the callbacks so they can
modify whatever they want, but I can also see the case made for passing
a copy of the event data in so that one plugin can't accidentally mess
up everyone else's data and violate the contract.

As for making things unnecessary, I hope that that is a rare case---what
might be considered unnecessary now may not be considered unnecessary in
the future.
As far as I can tell, it seems like the most likely uses of the stopPropagation function are when you are saying either "I want to be the only plugin to handle this, and I just hope that I'm the first one called" or "I know absolutely that no other plugin has any interest in handling this, so I'm going to just stop processing right now to save time." Either of these statements doesn't make much sense. You can't guarantee that you are the only one to handle a situation (that's the whole point of the event system---to let people have access to things), and you have no way of knowing that there will never be a plugin interested in processing things after you are done processing your bit.
e.g. a plugin to verify that a registration form was filled in by a real person not a spam bot. If it detects a spam bot, it determines the registration will not go ahead. It issues a stopPropagation and a preventDefault. If it passes the registration it does neither of these. A second plugin to handle a ban list does a similar thing based on its own decision making process. Swap the order and the same thing happens. 

Right.  Okay, good point.  That's another situation when you'd want to
call these functions.  And access control plugins (allow/deny decisions)
can be swapped around as much as you want, as long as the plugin does
nothing to allow and stops everything to deny.





2. As far as I can see, the preventDefault function has two uses:
(a) you are replacing the default action, so you don't want the default action clobbering what you're doing. (b) you have some reason to want _nothing_ done---the default or any other replacements for the default shouldn't happen.
In (a), with the proposed addition of a third "primary callback" advise, you would implement your callback as a primary callback and then use stopPropagation.
In (b), if you called preventDefault from a BEFORE callback, then that had better also prevent _any_ primary callbacks from being called, not just the default one. 


That is fair.
preventDefault in a BEFORE advisement should prevent both subsequent advisements being triggered as well as preventing the default action. preventDefault in the EVENT advisement will only prevent the default action. 

Would preventDefault in the EVENT advisement (nice name, by the way)
prevent other EVENT callbacks (i.e., would it act like stopPropagation?)

With the behavior of preventDefault in the BEFORE advisement, I vote for
a name-change to preventEvent or something like that.
stopPropagation should only prevent further propagation of the current advisement.
That does leave an opening for a new mechanism for EVENT handlers to prevent the AFTER advisement. I think it would be sufficient to add a parameter to preventDefault to indicate whether or not to trigger an AFTER advise. That would be like saying either: - I have handle the default action, there are results which should be made available via the AFTER advise. - I have cancelled the action. Nothing happened, ergo there is nothing to be after, so don't issue an AFTER advise.
For the purposes of simplicity and clarity it may be better to make three functions
preventDefault - cancel default action and EVENT advise. Includes one parameter to control issue of AFTER advise (defaults to true). stopPropagation - stop further propagation of this advise. Other advise are not affected. cancelEvent - stop propagation, do not issue any further advise, do not carry out the default action (when issued in BEFORE or EVENT advises). 

So, to clarify what you're saying, calling:

 - preventDefault in BEFORE, with the adviseAfter parameter true, will
continue processing BEFORE callbacks and then immediately skip to
processing AFTER callbacks.
 - preventDefault in BEFORE, with adviseAfter parameter false, will
continue processing BEFORE callbacks and then exit the event handling.
 - preventDefault in EVENT, with adviseAfter parameter true, will
continue processing EVENT callbacks, skip the default action, and then
start processing the AFTER callbacks.
 - preventDefault in EVENT, with adviseAfter parameter false, will
continue processing EVENT callbacks, skip the default action, and then
exit the event handling.
 - preventDefaultin AFTER callbacks does nothing.
 - stopPropagation, called in any advisement, will skip straight to the
next advisement.  That means if it is called in EVENT, then we skip the
rest of the EVENT callbacks _and_ the default action and skip straight
to AFTER callbacks.
 - cancelEvent immediately exits the event handling (after the current
callback finishes, of course).

Is that correct?

I don't like that the preventDefault action means something different if
it's called in BEFORE vs. EVENT.  I'll think about that one.

Another issue: suppose a BEFORE event calls preventDefault.  Then
another BEFORE event sets the preventDefault variable back to false
(yes, I know you shouldn't mess with the variables, but it's still
possible).  Should the default action be called?  I suppose the
preventDefault value is only checked once, right before calling the
default action, so the answer would be yes.  Should we do something to
enforce plugins not tampering with the preventDefault variables, or just
leave it as an understanding?


Thanks,

Jason

--
DokuWiki mailing list - more info at
http://wiki.splitbrain.org/wiki:mailinglist

Other related posts: