zapper-content.js: fix duplicate firing#323
Conversation
Resolve attaching three handlers that all trigger for a single tap, which onChild may run multiple times. Noticed behaviour when using "addManualRuleFromPrompt()", causing the prompt to appear twice on macOS when it should only be once. Kept 'click' as it works in iOS Safari, macOS Safari, and for accessibility (keyboard activation).
There was a problem hiding this comment.
Code Review
This pull request simplifies event handling in zapper-content.js by removing redundant pointerup and touchend event listeners from several UI buttons, including the undo, manual rule, done, parent, child, and hide buttons. A review comment suggests refactoring the associated event handler functions to use the existing interceptEvent helper to reduce code duplication and improve maintainability.
I am having trouble creating individual review comments. Click here to see my feedback.
wBlock Scripts (iOS)/Resources/zapper-content.js (436-437)
While you're fixing the multiple event firing, I noticed that the event handlers for the buttons (onUndo, onManualRule, onDone, onParent, onChild, onHide) all duplicate logic for stopping event propagation.
There's an existing helper function interceptEvent at line 804 that does exactly this.
You could refactor these handlers to use interceptEvent to reduce code duplication and improve readability.
For example, onUndo could be simplified to:
const onUndo = (e) => {
interceptEvent(e);
undoLastZap().catch(() => {});
};This would apply to all the other button handlers as well.
|
Looks good to me. This keeps the path for Safari and accessibility, and removes the duplicate and handlers that were causing repeat firing. |
Have not been able to test these changes, however, as I am unable to find a way to modify or overwrite Extension Scripts.