UI Taint w/ SpellFlash

[xemnosyst]

I recently made a change in Bitten's SpellFlash Library that one user reports causes issues when paired with nUI. Here are the error reported, and the changes I made. Maybe an nUI expert will understand how my changes would cause any issue?



The error:

1x [ADDON_ACTION_BLOCKED] AddOn "BittensSpellFlashLibrary" tried to call the protected function "nUI_TopRightBar_Button12:Show()".



My code:

local is = IsSpellOverlayed
local isnt = function() return false end
local show = ActionButton_ShowOverlayGlow
local noshow = function() end

function c.EnableDefaultProcHilighting()
	IsSpellOverlayed = is
	ActionButton_ShowOverlayGlow = show
	if LABFrame then 
		LABFrame:RegisterEvent("SPELL_ACTIVATION_OVERLAY_GLOW_SHOW") 
	end
end

function c.DisableDefaultProcHilighting()
	IsSpellOverlayed = isnt
	ActionButton_ShowOverlayGlow = noshow
	if LABFrame then 
		LABFrame:UnregisterEvent("SPELL_ACTIVATION_OVERLAY_GLOW_SHOW") 
	end
end

"LABFrame" is the eventFrame in LibActionButton.

[spiel2001]

It probably isn't your addon that's causing the taint. That's an action button taint that's been around forever and always gets blamed on the wrong addon. It just happens that it's showing up as a taint in nUI because nUI uses the action buttons.... i.e. something else caused the taint on my addon button, but your addon is taking the blame for it.

This taint bug has been around a very long time and is exceptionally difficult to pin down. I am working with Blizz to try and sort out what the cause is.

That said... it does look to me like you are tainting ActionButton_ShowOverlayGlow() because you are assigning it to a local variable, then assigning the local variable back. That will taint every time. Any time you assign to a Blizz function name or variable, you taint it. If it gets used during combat, it will taint the execution path.

I would suggest using

local function onOverlayGlow( blah blah blah )
    if procHighlighting then
        DoCustomHighlighting();
    end
end

hooksecurefunc( "ActionButton_ShowOverlayGlow", onOverlayGlow );

That is the secure way to modify Blizz behavior without tainting the execution path. It gets executed after the Blizz logic for the glow and allows you to overload its behavior with your own without adding taint to the execution path.

[xemnosyst]

Yeah I'm familiar with the trickiness of UI taint. Fortunately, my addon doesn't actually touch the UI, so in the past I have always just been able to say "Blizz blames me, but they are wrong" and go about my day without another thought.

I'm certainly open to finding a better way to accomplish my goal, but I'm not sure how to do so with the idea you propose. The trouble is that I'm not just trying to react the overlay, I'm trying to turn it off. In Cata, hooking it and then immediately calling ActionButton_HideOverlayGlow did the trick, but for whatever reason that doesn't work in MoP anymore. I was able to prevent some spells with ActionBarActionEventsFrame:UnregisterEvent("SPELL_ACTIVATION_OVERLAY_GLOW_SHOW"), but not all. Finally I found that making IsSpellOverlayed always return false did the trick.

I'm not sure you cared to read all that. Perhaps I'll try finding a way to only taint that one function & see if it makes a difference.

[spiel2001]

The glow overlay is an actual frame if my memory serves me correctly, yes?

If you want to turn it off, you could try:

RegisterStateDriver( "glowFrameName", "visibility", "hide" );

[xemnosyst]

Thank you for your help. I believe I took care of the issue. After playing around with it for a while I discovered that the original solution that was working in Cata also works in MoP ... except with LibActionButton. Because of your explanation that assigning to a Blizz variable causes taint, I was able to wade through the LibActionButton source and come up with a solution that doesn't make any such assignment.

[spiel2001]

Awesome.

Glad I could help in some small way.