Allow changing element root widget type, prevent use after free in script settings

apps/openmw/mwgui/settingswindow.cpp

@@ -207,9 +207,9 @@ namespace MWGui
         }
     }
 
-    SettingsWindow::SettingsWindow() :
+    SettingsWindow::SettingsWindow() : WindowBase("openmw_settings_window.layout")
-        WindowBase("openmw_settings_window.layout"),
+        , mKeyboardMode(true)
-        mKeyboardMode(true)
+        , mCurrentPage(-1)
     {
         bool terrain = Settings::Manager::getBool("distant terrain", "Terrain");
         const std::string widgetName = terrain ? "RenderingDistanceSlider" : "LargeRenderingDistanceSlider";
@@ -729,8 +729,8 @@ namespace MWGui
          
     void SettingsWindow::renderScriptSettings()
     {
-        while (mScriptView->getChildCount() > 0)
+        LuaUi::attachToWidget(mCurrentPage);
-            mScriptView->getChildAt(0)->detachFromWidget();
+        mCurrentPage = -1;
         mScriptList->removeAllItems();
         mScriptView->setCanvasSize({0, 0});
 
@@ -763,13 +763,13 @@ namespace MWGui
 
     void SettingsWindow::onScriptListSelection(MyGUI::Widget*, size_t index)
     {
-        while (mScriptView->getChildCount() > 0)
+        if (mCurrentPage >= 0)
-            mScriptView->getChildAt(0)->detachFromWidget();
+            LuaUi::attachToWidget(mCurrentPage);
+        mCurrentPage = -1;
         if (index >= mScriptList->getItemCount())
             return;
-        size_t scriptIndex = *mScriptList->getItemDataAt<size_t>(index);
+        mCurrentPage = *mScriptList->getItemDataAt<size_t>(index);
-        LuaUi::ScriptSettings script = LuaUi::scriptSettings()[scriptIndex];
+        LuaUi::attachToWidget(mCurrentPage, mScriptView);
-        LuaUi::attachToWidget(script, mScriptView);
         MyGUI::IntSize canvasSize;
         if (mScriptView->getChildCount() > 0)
             canvasSize = mScriptView->getChildAt(0)->getSize();

apps/openmw/mwgui/settingswindow.hpp

@@ -49,6 +49,7 @@ namespace MWGui
             MyGUI::Widget* mScriptBox;
             MyGUI::ScrollView* mScriptView;
             MyGUI::EditBox* mScriptDisabled;
+            int mCurrentPage;
 
             void onTabChanged(MyGUI::TabControl* _sender, size_t index);
             void onOkButtonClicked(MyGUI::Widget* _sender);

components/lua_ui/element.cpp

@@ -138,7 +138,7 @@ namespace LuaUi
         ext->setChildren(updateContent(ext->children(), layout.get<sol::object>(LayoutKeys::content)));
     }
 
-    void setLayer(WidgetExtension* ext, const sol::table& layout)
+    std::string setLayer(WidgetExtension* ext, const sol::table& layout)
     {
         MyGUI::ILayer* layerNode = ext->widget()->getLayer();
         std::string currentLayer = layerNode ? layerNode->getName() : std::string();
@@ -149,15 +149,18 @@ namespace LuaUi
         {
             MyGUI::LayerManager::getInstance().attachToLayerNode(newLayer, ext->widget());
         }
+        return newLayer;
     }
 
     std::map<Element*, std::shared_ptr<Element>> Element::sAllElements;
 
     Element::Element(sol::table layout)
-        : mRoot{ nullptr }
+        : mRoot(nullptr)
-        , mLayout{ std::move(layout) }
+        , mAttachedTo(nullptr)
-        , mUpdate{ false }
+        , mLayout(std::move(layout))
-        , mDestroy{ false }
+        , mLayer()
+        , mUpdate(false)
+        , mDestroy(false)
     {}
 
 
@@ -174,7 +177,8 @@ namespace LuaUi
         if (!mRoot)
         {
             mRoot = createWidget(mLayout);
-            setLayer(mRoot, mLayout);
+            mLayer = setLayer(mRoot, mLayout);
+            updateAttachment();
         }
     }
 
@@ -182,8 +186,17 @@ namespace LuaUi
     {
         if (mRoot && mUpdate)
         {
-            updateWidget(mRoot, mLayout);
+            if (mRoot->widget()->getTypeName() != widgetType(mLayout))
-            setLayer(mRoot, mLayout);
+            {
+                destroyWidget(mRoot);
+                mRoot = createWidget(mLayout);
+            }
+            else
+            {
+                updateWidget(mRoot, mLayout);
+            }
+            mLayer = setLayer(mRoot, mLayout);
+            updateAttachment();
         }
         mUpdate = false;
     }
@@ -195,4 +208,35 @@ namespace LuaUi
         mRoot = nullptr;
         sAllElements.erase(this);
     }
+
+    void Element::attachToWidget(MyGUI::Widget* w)
+    {
+        if (mAttachedTo && w)
+            throw std::logic_error("A UI element can't be attached to two widgets at once");
+        mAttachedTo = w;
+        updateAttachment();
+    }
+
+    void Element::updateAttachment()
+    {
+        if (!mRoot)
+            return;
+        if (mAttachedTo)
+        {
+            if (!mLayer.empty())
+                Log(Debug::Warning) << "Ignoring element's layer " << mLayer << " because it's attached to a widget";
+            if (mRoot->widget()->getParent() != mAttachedTo)
+            {
+                mRoot->widget()->attachToWidget(mAttachedTo);
+                mRoot->updateCoord();
+            }
+        }
+        else
+        {
+            if (mRoot->widget()->getParent() != nullptr)
+            {
+                mRoot->widget()->detachFromWidget();
+            }
+        }
+    }
 }

components/lua_ui/element.hpp

@@ -9,8 +9,10 @@ namespace LuaUi
     {
         static std::shared_ptr<Element> make(sol::table layout);
 
-        LuaUi::WidgetExtension* mRoot;
+        WidgetExtension* mRoot;
+        MyGUI::Widget* mAttachedTo;
         sol::table mLayout;
+        std::string mLayer;
         bool mUpdate;
         bool mDestroy;
 
@@ -22,9 +24,12 @@ namespace LuaUi
 
         friend void clearUserInterface();
 
+        void attachToWidget(MyGUI::Widget* w = nullptr);
+
         private:
             Element(sol::table layout);
             static std::map<Element*, std::shared_ptr<Element>> sAllElements;
+            void updateAttachment();
     };
 }
 

components/lua_ui/scriptsettings.cpp

@@ -26,12 +26,9 @@ namespace LuaUi
         allSettings.clear();
     }
 
-    void attachToWidget(const ScriptSettings& script, MyGUI::Widget* widget)
+    void attachToWidget(size_t index, MyGUI::Widget* widget)
     {
-        WidgetExtension* root = script.mElement->mRoot;
+        if (0 <= index && index < allSettings.size())
-        if (!root)
+            allSettings[index].mElement->attachToWidget(widget);
-            return;
-        root->widget()->attachToWidget(widget);
-        root->updateCoord();
     }
 }

components/lua_ui/scriptsettings.hpp

@@ -19,7 +19,7 @@ namespace LuaUi
     const std::vector<ScriptSettings>& scriptSettings();
     void registerSettings(const ScriptSettings& script);
     void clearSettings();
-    void attachToWidget(const ScriptSettings& script, MyGUI::Widget* widget);
+    void attachToWidget(size_t index, MyGUI::Widget* widget = nullptr);
 }
 
 #endif // !OPENMW_LUAUI_SCRIPTSETTINGS