[Libreoffice-commits] core.git: vcl/win

[Mike Kaganski (via logerrit)]

 vcl/win/window/salframe.cxx |   19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

New commits:
commit b094741e1d04f4e377c4a7d5c564e38d2f84c377
Author:     Mike Kaganski <mike.kaganski at collabora.com>
AuthorDate: Sun Aug 2 00:19:42 2020 +0300
Commit:     Mike Kaganski <mike.kaganski at collabora.com>
CommitDate: Wed Aug 5 12:04:40 2020 +0200

    tdf#135330: avoid replacing all settings when enabling IA2
    
    Call to ImplHandleGetObject may happen in the middle of any UI operation,
    including those that make use of references to parts of global settings,
    like this:
    
        const LocaleDataWrapper& rLocaleData = Application::GetSettings().GetUILocaleDataWrapper();
        setValue(*m_xCurrentWordFT, rCurrent.nWord, rLocaleData); // < here ImplHandleGetObject may be called
        setValue(*m_xCurrentCharacterFT, rCurrent.nChar, rLocaleData);
    
    If all settings get replaced, then LocaleDataWrapper gets destroyed, and
    the reference becomes invalid, and crashes in the next line.
    
    So first, the change makes the call only modify settings when it's needed;
    and second, it makes use of the implementation detail that aMisc modifies
    the shared data. The check after the change will catch if implementation
    changes so that this is no more true.
    
    Yet, the problem stays that using any references to global settings objects
    is unsafe, since any call to Application::SetSettings will invalidate them.
    
    Change-Id: I96d237ee57e80465fe52bc6bb6c149b087c89af9
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/99947
    Tested-by: Jenkins
    Reviewed-by: Mike Kaganski <mike.kaganski at collabora.com>

diff --git a/vcl/win/window/salframe.cxx b/vcl/win/window/salframe.cxx
index 287863129a9f..c433de48216d 100644
--- a/vcl/win/window/salframe.cxx
+++ b/vcl/win/window/salframe.cxx
@@ -5291,15 +5291,18 @@ static void ImplHandleIMENotify( HWND hWnd, WPARAM wParam )
 static bool
 ImplHandleGetObject(HWND hWnd, LPARAM lParam, WPARAM wParam, LRESULT & nRet)
 {
-    // IA2 should be enabled automatically
-    AllSettings aSettings = Application::GetSettings();
-    MiscSettings aMisc = aSettings.GetMiscSettings();
-    aMisc.SetEnableATToolSupport( true );
-    aSettings.SetMiscSettings( aMisc );
-    Application::SetSettings( aSettings );
-
     if (!Application::GetSettings().GetMiscSettings().GetEnableATToolSupport())
-        return false; // locked down somehow ?
+    {
+        // IA2 should be enabled automatically
+        AllSettings aSettings = Application::GetSettings();
+        MiscSettings aMisc = aSettings.GetMiscSettings();
+        aMisc.SetEnableATToolSupport(true);
+        // The above is enough, since aMisc changes the same shared ImplMiscData as used in global
+        // gettings, so no need to call aSettings.SetMiscSettings and Application::SetSettings
+
+        if (!Application::GetSettings().GetMiscSettings().GetEnableATToolSupport())
+            return false; // locked down somehow ?
+    }
 
     ImplSVData* pSVData = ImplGetSVData();