Skip to content

Commit 451a0ed

Browse files
committed
esc taint fix
1 parent f1cfb35 commit 451a0ed

File tree

3 files changed

+93
-29
lines changed

3 files changed

+93
-29
lines changed

.context/patterns.md

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,81 @@ end)
122122

123123
**When to Apply**: Any time you need to hook mouse events on buttons that use `ContainerFrameItemButtonTemplate` or any button where clicks can trigger protected functions.
124124

125+
### Pattern: Never Manipulate Blizzard Frames from OnHide/OnShow Scripts Triggered by UISpecialFrames
126+
**Problem**: Touching BankPanel or calling CloseBankFrame() from frame OnHide/OnShow scripts causes persistent taint that breaks UseContainerItem() for ALL containers (including backpack) after the bank is closed via ESC key.
127+
128+
**Why**: When ESC is pressed, WoW's engine calls `CloseSpecialWindows()` which iterates through `UISpecialFrames` and hides each frame. This hiding happens in a protected execution context. Any frame scripts (OnHide/OnShow) that execute during this process are still within the protected context. If these scripts touch Blizzard frames like BankPanel or call protected functions like CloseBankFrame(), they create persistent taint. Later, when the user opens their backpack and uses an item, Blizzard's protected `UseContainerItem()` calls `BankFrame:GetActiveBankType()` which reads from the tainted BankPanel, causing the action to be blocked.
129+
130+
**Solution Pattern**:
131+
1. **Never touch BankPanel in OnHide/OnShow scripts** - move all BankPanel manipulation to event handlers
132+
2. **Never call CloseBankFrame() in OnHide** - let Blizzard's CloseSpecialWindows handle it, or call it from event handlers
133+
3. **Use BANKFRAME_OPENED/CLOSED event handlers** for safe BankPanel manipulation (these run in addon context, not protected context)
134+
4. **Keep OnHide/OnShow scripts minimal** - only handle your own addon's UI cleanup (sounds, animations, frame hiding)
135+
136+
**Example**:
137+
```lua
138+
-- BAD: OnHide touches BankPanel (runs in protected context from UISpecialFrames)
139+
function bank.proto:OnHide()
140+
addon.ForceHideBlizzardBags()
141+
PlaySound(SOUNDKIT.IG_MAINMENU_CLOSE)
142+
143+
-- WRONG: These calls create persistent taint!
144+
if BankPanel then
145+
BankPanel:Hide() -- Taints BankPanel
146+
end
147+
if C_Bank then
148+
C_Bank.CloseBankFrame() -- Duplicate call in protected context
149+
end
150+
end
151+
152+
-- GOOD: OnHide only handles own UI, no Blizzard frame manipulation
153+
function bank.proto:OnHide()
154+
-- IMPORTANT: Do NOT touch BankPanel or call CloseBankFrame() here.
155+
-- OnHide runs in protected context when triggered by UISpecialFrames (ESC key).
156+
-- Any BankPanel manipulation here causes persistent taint.
157+
158+
addon.ForceHideBlizzardBags()
159+
PlaySound(SOUNDKIT.IG_MAINMENU_CLOSE)
160+
161+
if database:GetEnableBagFading() then
162+
self.bag.fadeOutGroup.callback = function()
163+
self.bag.fadeOutGroup.callback = nil
164+
ItemButtonUtil.TriggerEvent(ItemButtonUtil.Event.ItemContextChanged)
165+
end
166+
self.bag.fadeOutGroup:Play()
167+
else
168+
self.bag.frame:Hide()
169+
ItemButtonUtil.TriggerEvent(ItemButtonUtil.Event.ItemContextChanged)
170+
end
171+
end
172+
173+
-- GOOD: BankPanel manipulation in event handler (safe context)
174+
function addon.CloseBank(ctx, _, interactingFrame)
175+
if interactingFrame ~= nil then return end
176+
if addon.Bags.Bank then
177+
addon.Bags.Bank:Hide(ctx)
178+
addon.Bags.Bank:SwitchToBankAndWipe(ctx)
179+
end
180+
181+
-- Safe to hide BankPanel here - event handler runs in addon context
182+
if BankPanel then
183+
BankPanel:Hide()
184+
end
185+
186+
events:SendMessage(ctx, 'bags/BankClosed')
187+
end
188+
```
189+
190+
**When to Apply**:
191+
- Any time implementing custom bank frame behavior with UISpecialFrames registration
192+
- When hooking CloseSpecialWindows or handling BANKFRAME_OPENED/CLOSED events
193+
- Any frame that can be closed via ESC key and interacts with Blizzard's protected frames
194+
195+
**Related Files**:
196+
- bags/bank.lua:148-187 (OnHide method)
197+
- core/hooks.lua:111-134 (CloseSpecialWindows hook)
198+
- core/hooks.lua:138-153 (CloseBank event handler)
199+
125200
## State Management Across Events
126201

127202
### Pattern: Context Filter Propagation

bags/bank.lua

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -146,42 +146,24 @@ function bank.proto:OnShow(ctx)
146146
end
147147

148148
function bank.proto:OnHide()
149+
-- IMPORTANT: Do NOT touch BankPanel or call CloseBankFrame() here.
150+
-- OnHide runs in protected context when triggered by UISpecialFrames (ESC key).
151+
-- Any BankPanel manipulation here causes persistent taint that breaks UseContainerItem()
152+
-- for ALL containers (including backpack) after the bank is closed.
153+
-- See patterns.md for details.
154+
149155
addon.ForceHideBlizzardBags()
150156
PlaySound(SOUNDKIT.IG_MAINMENU_CLOSE)
151157

152158
-- Use fade animation if enabled
153159
if database:GetEnableBagFading() then
154160
self.bag.fadeOutGroup.callback = function()
155161
self.bag.fadeOutGroup.callback = nil -- Clean up callback
156-
157-
-- Hide BankPanel after fade completes to prevent taint
158-
if BankPanel then
159-
BankPanel:Hide()
160-
end
161-
162-
if C_Bank then
163-
C_Bank.CloseBankFrame()
164-
else
165-
CloseBankFrame()
166-
end
167-
168162
ItemButtonUtil.TriggerEvent(ItemButtonUtil.Event.ItemContextChanged)
169163
end
170164
self.bag.fadeOutGroup:Play()
171165
else
172166
self.bag.frame:Hide()
173-
174-
-- Hide BankPanel to prevent taint from affecting other container operations
175-
if BankPanel then
176-
BankPanel:Hide()
177-
end
178-
179-
if C_Bank then
180-
C_Bank.CloseBankFrame()
181-
else
182-
CloseBankFrame()
183-
end
184-
185167
ItemButtonUtil.TriggerEvent(ItemButtonUtil.Event.ItemContextChanged)
186168
end
187169
end

core/hooks.lua

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,11 +125,11 @@ function addon:CloseSpecialWindows(interactingFrame)
125125
end)
126126
end
127127
events:SendMessage(ctx, 'addon/CloseSpecialWindows')
128-
if C_Bank then
129-
C_Bank.CloseBankFrame()
130-
else
131-
CloseBankFrame()
132-
end
128+
129+
-- Don't call CloseBankFrame() here - Blizzard's CloseSpecialWindows() already
130+
-- handles closing the bank if it's open. Our BANKFRAME_CLOSED handler will
131+
-- clean up BankPanel safely.
132+
133133
events:SendMessageLater(ctx, 'bags/OpenClose')
134134
end
135135

@@ -142,5 +142,12 @@ function addon.CloseBank(ctx, _, interactingFrame)
142142
addon.Bags.Bank:Hide(ctx)
143143
addon.Bags.Bank:SwitchToBankAndWipe(ctx)
144144
end
145+
146+
-- Hide BankPanel in event handler context (safe from taint)
147+
-- This must happen AFTER the bank frame is hidden to prevent taint
148+
if BankPanel then
149+
BankPanel:Hide()
150+
end
151+
145152
events:SendMessage(ctx, 'bags/BankClosed')
146153
end

0 commit comments

Comments
 (0)