Bind TouchManager listeners without abort signals to survive iOS GC#21638
Bind TouchManager listeners without abort signals to survive iOS GC#21638nanalucky wants to merge 1 commit into
Conversation
…ive WebKit GC On iOS Safari, DOM listener registrations bound with a locally-held AbortSignal.any() composite can be silently dropped once garbage collection runs. When that hits the touchend/touchcancel cleanup listeners armed during a pinch while the capture-phase stopEvent listeners survive, the pinch never ends: every pointer event on the container is permanently swallowed and all annotation editing dies until the page is reloaded. This is likely the root cause of the stuck state reported in issue mozilla#19793. Bind the two transient listener sets plainly, track them in private fields and remove them explicitly (pointer-down cancel path, #onTouchEnd, destroy). If a fresh single-finger touchstart arrives while the pinch set is still armed (a missed end event), tear it down and fire onPinchEnd so the stuck state self-heals on the next touch. Verified on an iPhone: repeated draw + pinch-zoom cycles stay fully functional, where previously annotation interaction died permanently after one to four pinches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
WebKit silently drops DOM listeners bound with a locally-held AbortSignal.any() composite once GC runs. When that hit TouchManager's touchend/touchcancel cleanup while its capture-phase stopEvent listeners survived, a pinch never ended and every pointer event on the container was permanently swallowed — all annotation editing died until reload. Bind TouchManager's transient listener sets plainly with explicit removal (and recover if a pinch end was missed), and retain the composite signals returned by combinedSignal until they abort. Same fix submitted upstream: mozilla#21638 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Given that the primary development target for PDF.js is Firefox, why should we hurt readability/maintainability of the code just to hack-around what seems like a bug in iOS?
Note that we purposely converted the code-base to use AbortSignal in order to shorten/improve the code, and effectively reverting that seems quite unfortunate.
Besides, by just removing the AbortSignal handling like this patch does there's no longer any guarantee that closing the viewer removes all listeners.
Also, how much of this patch is AI generated?
|
Please open a detailed issue describing the problem instead (written in your own words and not just AI generated), since this patch unfortunately seems like the wrong approach. |
Are you sure about that ? The specs are pretty clear: You should report the bug to Apple. |
3 participants
On iOS Safari, DOM listener registrations bound with a locally-held AbortSignal.any() composite can be silently dropped once garbage collection runs. When that hits the touchend/touchcancel cleanup listeners armed during a pinch while the capture-phase stopEvent listeners survive, the pinch never ends: every pointer event on the container is permanently swallowed and all annotation editing dies until the page is reloaded. This is likely the root cause of the stuck state reported in issue #19793.
Bind the two transient listener sets plainly, track them in private fields and remove them explicitly (pointer-down cancel path, #onTouchEnd, destroy). If a fresh single-finger touchstart arrives while the pinch set is still armed (a missed end event), tear it down and fire onPinchEnd so the stuck state self-heals on the next touch.
Verified on an iPhone: repeated draw + pinch-zoom cycles stay fully functional, where previously annotation interaction died permanently after one to four pinches.