Make Open Mail recovery lazy and bump to 1.4.12

- Leave the existing Open Mail button untouched during normal mailbox opens
- Only run full recovery if the button is missing, hidden or re-parented
- Match Otari-style normal behavior while keeping compatibility recovery
- Keep Open All, inbox polling, sending and logging unchanged
- Bump version to 1.4.12

AUDIT.md

@@ -17,6 +17,28 @@ Target: Turtle WoW 1.18.x / Octo client.
 - **Calendar tooltip:** disabled/empty calendar days no longer concatenate a nil/stale `mails` value.
 - **OnUpdate rebinding:** `TurtleMail:init()` binds the original `on_update` function before the compatibility file loads; the patch explicitly rebinds the frame script so the safer handler is actually used.
 
+
+## Fixed in 1.4.8
+
+- **`MailMailButton` nil crash:** upstream stores the real `SendMailMailButton` in a loose global named `MailMailButton`, then replaces the Blizzard global with a proxy table. If the saved global is missing or another UI addon changes it, `SendMailFrame_CanSend()` and `sendmail_clear()` crash while calling `Enable()` / `Disable()`. The compatibility layer now preserves a strong reference to the real button before the proxy swap, restores it when the mailbox opens, replaces the fragile CanSend hook, and uses a guarded clear routine.
+- **Paired subject edit-box hardening:** the same proxy pattern is used for `MailSubjectEditBox`; the real edit box is now preserved and restored alongside the send button to avoid the equivalent follow-up failure.
+
+## Fixed in 1.4.9
+
+- **Missing `Open Mail` button:** upstream creates `TurtleMailOpenMailButton` only once in `inbox_load()` and assumes it will remain parented, positioned, and visible forever. If that creation path is interrupted or another mailbox/UI addon hides or re-parents the button, TurtleMail never recovers it. The compatibility layer now uses an idempotent inbox loader and an `ensure_open_mail_button()` recovery path that recreates the button if needed and restores its parent/anchor/text/click handler/frame level.
+
+## Fixed in 1.4.10
+
+- **First mailbox-open freeze/stutter after the 1.4.9 recovery fix:** 1.4.9 called `ensure_open_mail_button()` on every `MAIL_INBOX_UPDATE`. Initial inbox population can emit a burst of those events, so the button was repeatedly re-parented, re-anchored, re-labeled, resized, and given a new frame level. 1.4.10 removes that work from `MAIL_INBOX_UPDATE` and makes `ensure_open_mail_button()` state-aware, so it only mutates the UI when the button is missing, hidden, moved, re-parented, or otherwise incorrect.
+
+## Fixed in 1.4.11
+
+- **Residual mailbox hitch / excessive garbage generation:** the upstream `GetContainerItemInfo` hook used `pack()` + `unpack()` for every bag query, allocating a temporary table each time. Bag/mailbox refreshes can call this path very frequently and trigger avoidable garbage collection pauses. 1.4.11 replaces the installed hook after login with a fixed-local wrapper that performs no per-call table allocation. The native return count is detected once and the exact arity is preserved for signatures of up to 12 return values; unusual signatures fall back to the previous hook.
+
+## Fixed in 1.4.12
+
+- **Open Mail recovery overhead on mailbox open:** 1.4.11 still called the full `ensure_open_mail_button()` routine from `MAIL_SHOW()`. Even when the button was healthy, that path inspected parent, anchor, label, script, size, frame level and visibility. 1.4.12 keeps the normal path Otari-like: if `TurtleMailOpenMailButton` already exists, is parented to `InboxFrame`, and is shown, `MAIL_SHOW()` leaves it completely untouched. The full recovery routine now runs only if the button is missing, hidden, or re-parented. No inbox polling, Open All, sending, logging, or mail-processing logic is changed.
+
 ## Remaining architectural risks upstream
 
 These are intentionally not rewritten in the compatibility layer because changing them would be much more invasive and could alter mail behavior:
@@ -38,4 +60,4 @@ The compatibility build was tested in-game on Turtle WoW 1.18.x and the reported
 
 ## Status
 
-**1.4.6 is the current stable release.** The fixes remain isolated in `TurtleMailFix.lua` so upstream code stays easy to compare and future upstream changes remain easier to merge.
+**1.4.12 is the current test build pending in-game validation.** The fixes remain isolated in `TurtleMailFix.lua` so upstream code stays easy to compare and future upstream changes remain easier to merge.

README.md

@@ -76,7 +76,7 @@ When enabled, TurtleMail records sent and received mail information in its dedic
 
 ## 🔧 Turtle WoW compatibility
 
-Version **1.4.7** extends the compatibility and safety layer for Turtle WoW 1.18.x environments.
+Version **1.4.12** extends the compatibility, safety, and performance layer for Turtle WoW 1.18.x environments.
 
 The compatibility work is isolated in `TurtleMailFix.lua` so the original TurtleMail code remains easier to compare with upstream versions.
 
@@ -95,6 +95,10 @@ The compatibility work is isolated in `TurtleMailFix.lua` so the original Turtle
 - Fixed calendar tooltip nil/stale-value cases.
 - Rebound the safer `OnUpdate` handler correctly after loading the compatibility layer.
 - Prevented the send queue from remaining stuck when an attachment becomes unavailable during sending.
+- Fixed `MailMailButton` nil crashes while enabling/disabling or clearing the send form, and hardened the paired subject edit-box reference.
+- Restored the missing **Open Mail** button with an idempotent recovery path.
+- Reduced first-open mailbox stutter by removing Open Mail layout repair from `MAIL_INBOX_UPDATE` and only changing the button when its state is actually wrong.
+- Made Open Mail recovery fully lazy on mailbox open: normal openings now leave the existing button completely untouched, matching Otari-style behavior; full repair only runs if the button is missing, hidden, or re-parented.
 
 The complete technical audit is available in [`AUDIT.md`](AUDIT.md).
 
@@ -127,13 +131,15 @@ TurtleMail includes localization support for:
 
 ## 📜 Version
 
-Current stable version:
+Current test version:
 
-**1.4.7**
+**1.4.11**
 
 Based on upstream **TurtleMail 1.4.5**.
 
-Version 1.4.7 has been validated in-game on Turtle WoW 1.18.x without Lua errors in the tested setup.
+Version 1.4.10 keeps the **Open Mail** recovery from 1.4.9 but makes it lightweight: the button is repaired on load/mailbox open only when needed, instead of being re-parented and re-anchored on every inbox refresh. This avoids repeated layout work during the initial mailbox update burst.
+
+Version 1.4.11 also removes the per-call `pack()`/`unpack()` allocation from TurtleMail's `GetContainerItemInfo` hook. The native return count is detected once at login and preserved exactly (up to 12 values), while normal bag queries use fixed local values. This reduces garbage collection pressure during bag/mailbox refreshes without changing mail behavior.
 
 ## 🖼️ Screenshots
 

TurtleMail.toc

@@ -2,7 +2,7 @@
 ## Interface: 11200
 ## Title: |cffabd473Turtle|cffffffffMail
 ## Author: shirsig/sica, Dusk-92 (compatibility fixes)
-## Version: 1.4.7
+## Version: 1.4.12
 ## Notes: Mailbox enhancement - Turtle WoW 1.18.x compatibility
 ## SavedVariables: TurtleMail_AutoCompleteNames
 ## SavedVariablesPerCharacter: TurtleMail_To TurtleMail_Point TurtleMail_Log

TurtleMailFix.lua

@@ -8,7 +8,7 @@ local m = TurtleMail
 local getn = table.getn
 local function pack( ... ) return arg end
 
-m.compat_version = "1.4.7"
+m.compat_version = "1.4.12"
 
 local DEFAULT_SENT_FILTERS = {
   Money = 1,
@@ -110,6 +110,190 @@ local function ensure_horizontal_bars()
   return m.api.MailHorizontalBarLeft and m.api.MailHorizontalBarRight
 end
 
+-- Upstream keeps the real Send button and subject edit box in ad-hoc globals
+-- (MailMailButton / MailSubjectEditBox), then replaces the Blizzard globals with
+-- proxy tables. If another addon changes those globals, or sendmail_load() runs
+-- with an already-proxied widget, the saved globals can become nil and later
+-- calls crash. Preserve strong references before the upstream swap happens.
+local preserved_send_mail_button = m.api.SendMailMailButton
+local preserved_subject_edit_box = m.api.SendMailSubjectEditBox
+
+local function is_named_widget( widget, expected_name )
+  if not widget or type( widget.GetName ) ~= "function" then return false end
+  return widget:GetName() == expected_name
+end
+
+local function ensure_send_mail_button()
+  local button = preserved_send_mail_button
+
+  if not is_named_widget( button, "SendMailMailButton" ) then
+    button = m.real_send_mail_button
+  end
+  if not is_named_widget( button, "SendMailMailButton" ) then
+    button = MailMailButton
+  end
+  if not is_named_widget( button, "SendMailMailButton" ) then
+    button = m.api.SendMailMailButton
+  end
+
+  if is_named_widget( button, "SendMailMailButton" ) then
+    preserved_send_mail_button = button
+    m.real_send_mail_button = button
+    MailMailButton = button
+    return button
+  end
+
+  return nil
+end
+
+local function ensure_subject_edit_box()
+  local edit_box = preserved_subject_edit_box
+
+  if not is_named_widget( edit_box, "SendMailSubjectEditBox" ) then
+    edit_box = m.real_subject_edit_box
+  end
+  if not is_named_widget( edit_box, "SendMailSubjectEditBox" ) then
+    edit_box = MailSubjectEditBox
+  end
+  if not is_named_widget( edit_box, "SendMailSubjectEditBox" ) then
+    edit_box = m.api.SendMailSubjectEditBox
+  end
+
+  if is_named_widget( edit_box, "SendMailSubjectEditBox" ) then
+    preserved_subject_edit_box = edit_box
+    m.real_subject_edit_box = edit_box
+    MailSubjectEditBox = edit_box
+    return edit_box
+  end
+
+  return nil
+end
+
+local function restore_real_mail_widgets_for_upstream()
+  local button = ensure_send_mail_button()
+  local edit_box = ensure_subject_edit_box()
+
+  -- Only restore the Blizzard globals immediately before upstream sendmail_load().
+  -- That function intentionally replaces them with proxy tables afterwards.
+  if button then m.api.SendMailMailButton = button end
+  if edit_box then m.api.SendMailSubjectEditBox = edit_box end
+end
+
+-- The upstream Open Mail button is created only once in inbox_load(). If that
+-- creation is skipped/interrupted, or another UI addon hides/re-parents the
+-- button afterwards, TurtleMail never recreates it. Keep the button recoverable,
+-- but only mutate its layout when something is actually wrong. This avoids doing
+-- repeated SetParent/ClearAllPoints/SetPoint work during inbox refresh bursts.
+local function ensure_open_mail_button()
+  local inbox = m.api.InboxFrame
+  if not inbox or not m.api.CreateFrame then return nil end
+
+  local button = m.api.TurtleMailOpenMailButton
+  local created = false
+  if not button or type( button.SetPoint ) ~= "function" then
+    button = m.api.CreateFrame( "Button", "TurtleMailOpenMailButton", inbox, "UIPanelButtonTemplate" )
+    m.api.TurtleMailOpenMailButton = button
+    created = true
+  end
+
+  local parent_changed = false
+  if button.GetParent and button.SetParent then
+    if button:GetParent() ~= inbox then
+      button:SetParent( inbox )
+      parent_changed = true
+    end
+  elseif created and button.SetParent then
+    button:SetParent( inbox )
+    parent_changed = true
+  end
+
+  local needs_anchor = created or parent_changed
+  if not needs_anchor and button.GetPoint then
+    local point, relative_to, relative_point, x, y = button:GetPoint()
+    needs_anchor = point ~= "BOTTOM"
+      or (relative_to and relative_to ~= inbox)
+      or relative_point ~= "BOTTOM"
+      or x ~= -10
+      or y ~= 90
+  end
+
+  if needs_anchor then
+    if button.ClearAllPoints then button:ClearAllPoints() end
+    button:SetPoint( "BOTTOM", inbox, "BOTTOM", -10, 90 )
+  end
+
+  local wanted_text = m.api.OPENMAIL or "Open Mail"
+  local text_changed = created
+  if button.GetText then
+    text_changed = button:GetText() ~= wanted_text
+  end
+  if text_changed and button.SetText then
+    button:SetText( wanted_text )
+  end
+
+  -- Recalculate dimensions only when the button was created or its label changed.
+  if created or text_changed then
+    local width = 120
+    local font_string = button.GetFontString and button:GetFontString()
+    if font_string and font_string.GetStringWidth then
+      width = math.max( 120, 30 + (font_string:GetStringWidth() or 0) )
+    end
+    if button.SetWidth then button:SetWidth( width ) end
+    if button.SetHeight then button:SetHeight( 25 ) end
+  end
+
+  if button.SetScript then
+    local onclick = button.GetScript and button:GetScript( "OnClick" ) or nil
+    if created or onclick ~= m.inbox_open_all then
+      button:SetScript( "OnClick", m.inbox_open_all )
+    end
+  end
+
+  if inbox.GetFrameLevel and button.SetFrameLevel then
+    local target_level = (inbox:GetFrameLevel() or 0) + 5
+    local current_level = button.GetFrameLevel and button:GetFrameLevel() or nil
+    if current_level ~= target_level then
+      button:SetFrameLevel( target_level )
+    end
+  end
+
+  if button.Enable then
+    if not button.IsEnabled or not button:IsEnabled() then
+      button:Enable()
+    end
+  end
+  if button.Show then
+    if not button.IsShown or not button:IsShown() then
+      button:Show()
+    end
+  end
+  return button
+end
+
+-- Replace the fragile one-shot inbox loader with an idempotent version. This
+-- preserves upstream behavior while making Open Mail recoverable instead of
+-- relying on the initial CreateFrame call succeeding forever.
+function m.inbox_load()
+  if m.api.InboxFrame and m.api.InboxFrame.EnableMouse then
+    m.api.InboxFrame:EnableMouse( false )
+  end
+
+  ensure_open_mail_button()
+
+  for i = 1, 7 do
+    local auction_texture = m.api[ "TurtleMailAuctionIcon" .. i .. "Texture" ]
+    local returned_texture = m.api[ "TurtleMailReturnedArrow" .. i .. "Texture" ]
+    local color = m.api.NORMAL_FONT_COLOR
+
+    if color and auction_texture and auction_texture.SetVertexColor then
+      auction_texture:SetVertexColor( color.r, color.g, color.b )
+    end
+    if color and returned_texture and returned_texture.SetVertexColor then
+      returned_texture:SetVertexColor( color.r, color.g, color.b )
+    end
+  end
+end
+
 -- SavedVariables are not guaranteed to have a valid schema. Repair them before
 -- the original handlers touch nested fields.
 do
@@ -166,7 +350,50 @@ do
   local original = m.sendmail_load
   function m.sendmail_load()
     ensure_horizontal_bars()
-    if original then return original() end
+    restore_real_mail_widgets_for_upstream()
+
+    if original then
+      local result = original()
+      -- Upstream has now installed its proxy globals. Re-assert the private
+      -- references used by the legacy MailMailButton/MailSubjectEditBox code.
+      ensure_send_mail_button()
+      ensure_subject_edit_box()
+      return result
+    end
+  end
+end
+
+-- Replace the fragile upstream CanSend hook. The original directly calls the
+-- global MailMailButton and crashes when that saved global is nil.
+if m.hooks then
+  m.hook.SendMailFrame_CanSend = function()
+    local button = ensure_send_mail_button()
+    if not button then return end
+
+    local name_box = m.api.SendMailNameEditBox
+    local send_money_button = m.api.SendMailSendMoneyButton
+    local send_money_frame = m.api.SendMailMoney
+
+    local recipient = name_box and name_box.GetText and name_box:GetText() or ""
+    local attached = m.sendmail_num_attachments and m.sendmail_num_attachments() or 0
+    local price = m.api.GetSendMailPrice and (tonumber( m.api.GetSendMailPrice() ) or 0) or 0
+    local player_money = m.api.GetMoney and (tonumber( m.api.GetMoney() ) or 0) or 0
+    local money = 0
+
+    if send_money_button and send_money_button.GetChecked and send_money_button:GetChecked()
+        and m.api.MoneyInputFrame_GetCopper and send_money_frame then
+      money = tonumber( m.api.MoneyInputFrame_GetCopper( send_money_frame ) ) or 0
+    end
+
+    local can_send = not m.sendmail_sending
+        and string.len( recipient ) > 0
+        and money + price * math.max( 1, attached ) <= player_money
+
+    if can_send then
+      button:Enable()
+    else
+      button:Disable()
+    end
   end
 end
 
@@ -178,6 +405,67 @@ if m.hooks and m.hooks.SendMailFrame_Update then
   end
 end
 
+-- Ensure the legacy click handler sees a valid subject edit box and real send
+-- button before it reads them. This protects the same upstream proxy hack from
+-- producing a follow-up MailSubjectEditBox nil error.
+do
+  local original = m.send_mail_button_onclick
+  function m.send_mail_button_onclick()
+    ensure_send_mail_button()
+    ensure_subject_edit_box()
+    if original then return original() end
+  end
+end
+
+-- Replace the upstream clear routine because it directly dereferences
+-- MailMailButton and MailSubjectEditBox. Keep its behavior, but guard every UI
+-- object so mailbox replacements cannot turn cleanup into another Lua error.
+function m.sendmail_clear()
+  local any_item
+
+  for i = 1, 21 do
+    local attachment = m.api[ "MailAttachment" .. i ]
+    if attachment then
+      any_item = any_item or attachment.item
+      attachment.item = nil
+    end
+  end
+
+  if any_item and m.api.ClearCursor and m.api.PickupContainerItem then
+    m.api.ClearCursor()
+    m.api.PickupContainerItem( unpack( any_item ) )
+    m.api.ClearCursor()
+  end
+
+  local button = ensure_send_mail_button()
+  if button and button.Disable then button:Disable() end
+
+  local name_box = m.api.SendMailNameEditBox
+  if name_box then
+    if name_box.SetText then name_box:SetText( "" ) end
+    if name_box.SetFocus then name_box:SetFocus() end
+  end
+
+  local subject = ensure_subject_edit_box()
+  if subject and subject.SetText then subject:SetText( "" ) end
+
+  if m.api.SendMailBodyEditBox and m.api.SendMailBodyEditBox.SetText then
+    m.api.SendMailBodyEditBox:SetText( "" )
+  end
+
+  if m.api.MoneyInputFrame_ResetMoney and m.api.SendMailMoney then
+    m.api.MoneyInputFrame_ResetMoney( m.api.SendMailMoney )
+  end
+
+  if m.api.SendMailRadioButton_OnClick then
+    m.api.SendMailRadioButton_OnClick( 1 )
+  end
+
+  if m.api.SendMailFrame_Update then
+    m.api.SendMailFrame_Update()
+  end
+end
+
 -- Do not assume MailFrame exists during bag events.
 function m.BAG_UPDATE()
   if m.api.MailFrame and m.api.MailFrame:IsVisible() and m.api.SendMailFrame_Update then
@@ -187,6 +475,23 @@ end
 
 -- Safer MAIL_SHOW: package-button regions differ between UI replacements.
 function m.MAIL_SHOW()
+  -- Re-assert the legacy send-widget references whenever the mailbox opens.
+  -- Open Mail follows the lighter Otari-style path: when the existing button
+  -- is still shown and parented to InboxFrame, do nothing at all. Only invoke
+  -- the full recovery routine if the button is missing, hidden, or re-parented.
+  ensure_send_mail_button()
+  ensure_subject_edit_box()
+
+  local open_mail_button = m.api.TurtleMailOpenMailButton
+  local open_mail_ok = open_mail_button
+    and type( open_mail_button.SetPoint ) == "function"
+    and (not open_mail_button.GetParent or open_mail_button:GetParent() == m.api.InboxFrame)
+    and (not open_mail_button.IsShown or open_mail_button:IsShown())
+
+  if not open_mail_ok then
+    ensure_open_mail_button()
+  end
+
   if not m.api.MailFrame then return end
 
   if m.api.TurtleMail_Point then
@@ -616,3 +921,83 @@ do
     end
   end
 end
+
+-- Performance compatibility fix inspired by Otari98/TurtleMail: the upstream
+-- GetContainerItemInfo hook builds a temporary table with pack()/unpack() on
+-- every bag query. Bag and mailbox refreshes can call this hook very frequently,
+-- creating avoidable garbage and occasional GC hitches. Detect the native return
+-- count once after the normal PLAYER_LOGIN hook setup, then use a fixed-local
+-- wrapper with no per-call temporary table. Exact return arity is preserved for
+-- up to 12 values so extended/custom clients remain compatible; if detection is
+-- inconclusive, the existing upstream hook is left untouched.
+do
+  local original_player_login = m.PLAYER_LOGIN
+  local legacy_hook = m.hooks and m.hooks.GetContainerItemInfo or nil
+  local detected_return_count
+
+  local function count_returns( ... )
+    return arg.n or getn( arg )
+  end
+
+  local function detect_container_info_return_count()
+    local fn = m.orig and m.orig.GetContainerItemInfo
+    if type( fn ) ~= "function" then return nil end
+
+    -- Prefer an occupied inventory slot so clients that return no values for an
+    -- empty slot cannot make us detect the wrong arity.
+    if m.api.GetContainerNumSlots then
+      for bag = 0, 4 do
+        local slots = tonumber( m.api.GetContainerNumSlots( bag ) ) or 0
+        for slot = 1, slots do
+          local texture = fn( bag, slot )
+          if texture then
+            return count_returns( fn( bag, slot ) )
+          end
+        end
+      end
+    end
+
+    -- A normal character should always have at least one item, but retain a
+    -- conservative fallback for unusual test clients or empty inventories.
+    local count = count_returns( fn( 0, 1 ) )
+    if count and count > 0 then return count end
+    return nil
+  end
+
+  local function optimized_get_container_item_info( bag, slot )
+    local r1, r2, r3, r4, r5, r6, r7, r8, r9, r10, r11, r12 =
+      m.orig.GetContainerItemInfo( bag, slot )
+
+    if not r3 and m.sendmail_attached( bag, slot ) then
+      r3 = 1
+    end
+
+    if detected_return_count == 1 then return r1 end
+    if detected_return_count == 2 then return r1, r2 end
+    if detected_return_count == 3 then return r1, r2, r3 end
+    if detected_return_count == 4 then return r1, r2, r3, r4 end
+    if detected_return_count == 5 then return r1, r2, r3, r4, r5 end
+    if detected_return_count == 6 then return r1, r2, r3, r4, r5, r6 end
+    if detected_return_count == 7 then return r1, r2, r3, r4, r5, r6, r7 end
+    if detected_return_count == 8 then return r1, r2, r3, r4, r5, r6, r7, r8 end
+    if detected_return_count == 9 then return r1, r2, r3, r4, r5, r6, r7, r8, r9 end
+    if detected_return_count == 10 then return r1, r2, r3, r4, r5, r6, r7, r8, r9, r10 end
+    if detected_return_count == 11 then return r1, r2, r3, r4, r5, r6, r7, r8, r9, r10, r11 end
+    if detected_return_count == 12 then return r1, r2, r3, r4, r5, r6, r7, r8, r9, r10, r11, r12 end
+
+    -- Should not normally be reached; keep the previous behavior if a custom
+    -- client exposes an unexpected signature.
+    if legacy_hook then return legacy_hook( bag, slot ) end
+    return r1, r2, r3, r4, r5
+  end
+
+  function m.PLAYER_LOGIN()
+    if original_player_login then original_player_login() end
+
+    detected_return_count = detect_container_info_return_count()
+    if detected_return_count and detected_return_count >= 1 and detected_return_count <= 12 then
+      m.api.GetContainerItemInfo = optimized_get_container_item_info
+    end
+  end
+end
+