Delegate trust check to action handler #21553
Conversation
dreamofabear
force-pushed
the
action-handler-delegate-trust-check
branch
from
702acf1
to
52f7ba4
Compare
| @@ -775,7 +779,7 @@ export class ActionService { | |||
| * @return {boolean} | |||
| * @private | |||
There was a problem hiding this comment.
Curious, why this change? Is a trailing underscore no longer going to be used to identify @private? You're consistent as you introduced isAmpTagName which follows the same naming scheme and is private.
There was a problem hiding this comment.
I think it's more important for class functions than module-level functions. I.e. it's possible to accidentally invoke a private class function but not a module-level function that's not exported.
| // Dequeue the current queue. | ||
| if (isArray(currentQueue)) { | ||
| const queuedInvocations = target[ACTION_QUEUE_]; | ||
| if (isArray(queuedInvocations)) { |
There was a problem hiding this comment.
nit. not related to your change but is the isArray check for queuedInvocations is not necessary here. It is either null or an Array based on the type annotation associated with it and if so then checking for its presence would suffice. Just if(queuedInvocations) vs if (isArray(queuedInvocations))
There was a problem hiding this comment.
My guess is it's a safety measure in case some other code accidentally sets the same property on the element.
| @@ -775,7 +779,7 @@ export class ActionService { | |||
| * @return {boolean} | |||
| * @private | |||
There was a problem hiding this comment.
I think it's more important for class functions than module-level functions. I.e. it's possible to accidentally invoke a private class function but not a module-level function that's not exported.
| // Dequeue the current queue. | ||
| if (isArray(currentQueue)) { | ||
| const queuedInvocations = target[ACTION_QUEUE_]; | ||
| if (isArray(queuedInvocations)) { |
There was a problem hiding this comment.
My guess is it's a safety measure in case some other code accidentally sets the same property on the element.
This reverts commit 391a431.
)" (ampproject#21563)" This reverts commit d96498b.
Prerequisite for #21322.
For action handlers of non-AMP elements, remove the
ActionTrustcheck from registration time and allow the handler itself to do the check instead.This is a simple solution to support both HIGH and LOW trust actions in a single handler.
/to @alabiaga