Bug fix: Focus was lost after checking for updates

Fixed by removing the guarding code that prevented actions with the active window while checking for updates. The code was there likely because the previous VCL HTTP implementation pumped the Windows message queue, what the current neon implementation does not. Guard was added for the main window anyway (although likely not needed).

Source commit: 67a9cc786139ee7e42349596637b5470d6bdd0d9

source/forms/CustomScpExplorer.cpp

@@ -9255,7 +9255,16 @@ void __fastcall TCustomScpExplorerForm::UpdatesNoteClicked(TObject * /*Sender*/)
   if (CanDisplay)
   {
     Configuration->Usage->Inc(L"UpdateNotificationsClicked");
-    CheckForUpdates(true);
+    // Probably not needed as CheckForUpdates is blocking since we started using neon for HTTP implementation.
+    NonVisualDataModule->StartBusy();
+    try
+    {
+      CheckForUpdates(true);
+    }
+    __finally
+    {
+      NonVisualDataModule->EndBusy();
+    }
   }
   else
   {

source/windows/Setup.cpp

@@ -1612,148 +1612,130 @@ static void __fastcall InsertDonateLink(void * /*Data*/, TObject * Sender)
 //---------------------------------------------------------------------------
 bool __fastcall CheckForUpdates(bool CachedResults)
 {
-  TCustomForm * ActiveForm = Screen->ActiveCustomForm;
-
   bool Result = false;
   TOperationVisualizer Visualizer;
 
-  try
+  TUpdatesConfiguration Updates = WinConfiguration->Updates;
+  bool Cached =
+    Updates.HaveValidResultsForVersion(Configuration->CompoundVersion) &&
+    CachedResults;
+  if (!Cached)
   {
-    if (ActiveForm)
-    {
-      DebugAssert(ActiveForm->Enabled);
-      ActiveForm->Enabled = false;
-    }
+    DoQueryUpdates(WinConfiguration->CollectUsage);
+    // reread new data
+    Updates = WinConfiguration->Updates;
+  }
 
-    TUpdatesConfiguration Updates = WinConfiguration->Updates;
-    bool Cached =
-      Updates.HaveValidResultsForVersion(Configuration->CompoundVersion) &&
-      CachedResults;
-    if (!Cached)
-    {
-      DoQueryUpdates(WinConfiguration->CollectUsage);
-      // reread new data
-      Updates = WinConfiguration->Updates;
-    }
+  if (!Updates.ShownResults)
+  {
+    Updates.ShownResults = true;
+    WinConfiguration->Updates = Updates;
+  }
+  DebugAssert(Updates.HaveResults);
 
-    if (!Updates.ShownResults)
-    {
-      Updates.ShownResults = true;
-      WinConfiguration->Updates = Updates;
-    }
-    DebugAssert(Updates.HaveResults);
+  UnicodeString Message;
+  bool New;
+  TQueryType Type;
+  GetUpdatesMessage(Message, New, Type, true);
 
-    UnicodeString Message;
-    bool New;
-    TQueryType Type;
-    GetUpdatesMessage(Message, New, Type, true);
+  Configuration->Usage->Inc(L"UpdateDisplays");
+  if (New)
+  {
+    Configuration->Usage->Inc(L"UpdateDisplaysNew");
+  }
 
-    Configuration->Usage->Inc(L"UpdateDisplays");
-    if (New)
-    {
-      Configuration->Usage->Inc(L"UpdateDisplaysNew");
-    }
+  if ((double(Updates.Period) > 0) &&
+      // do not show next check time, if we have new version info
+      !New)
+  {
+    Message += L"\n\n" +
+      FMTLOAD(UPDATE_NEXT, (FormatDateTime("ddddd", Updates.LastCheck + Updates.Period)));
+  }
+  else if (New)
+  {
+    UnicodeString Version = Configuration->GetVersionStrHuman();
+    Message += L"\n\n" + FMTLOAD(UPDATE_CURRENT, (Version));
+  }
 
-    if ((double(Updates.Period) > 0) &&
-        // do not show next check time, if we have new version info
-        !New)
-    {
-      Message += L"\n\n" +
-        FMTLOAD(UPDATE_NEXT, (FormatDateTime("ddddd", Updates.LastCheck + Updates.Period)));
-    }
-    else if (New)
-    {
-      UnicodeString Version = Configuration->GetVersionStrHuman();
-      Message += L"\n\n" + FMTLOAD(UPDATE_CURRENT, (Version));
-    }
+  int Answers = qaOK |
+    // show "what's new" button only when change list URL was not provided in results
+    FLAGMASK(New && Updates.Results.NewsUrl.IsEmpty(), qaAll) |
+    FLAGMASK(New, qaCancel) |
+    FLAGMASK(!Updates.Results.Url.IsEmpty(), qaYes);
+  TQueryButtonAlias Aliases[4];
+  Aliases[0].Button = qaYes;
+  if (Updates.Results.UrlButton.IsEmpty())
+  {
+    Aliases[0].Alias = LoadStr(UPDATE_URL_BUTTON);
+  }
+  else
+  {
+    Aliases[0].Alias = Updates.Results.UrlButton;
+  }
+  Aliases[1].Button = qaAll;
+  Aliases[1].Alias = LoadStr(WHATS_NEW_BUTTON);
+  Aliases[1].OnSubmit = MakeMethod<TButtonSubmitEvent>(NULL, OpenHistory);
+  Aliases[2].Button = qaCancel;
+  Aliases[2].Alias = Vcl_Consts_SMsgDlgClose;
+  // Used only when New == true, see AliasesCount below
+  Aliases[3].Button = qaOK;
+  Aliases[3].Alias = LoadStr(UPGRADE_BUTTON);
+  if (!Updates.Results.DownloadUrl.IsEmpty())
+  {
+    Aliases[3].OnSubmit = MakeMethod<TButtonSubmitEvent>(NULL, DownloadUpdate);
+    Aliases[3].ElevationRequired = true;
+  }
 
-    int Answers = qaOK |
-      // show "what's new" button only when change list URL was not provided in results
-      FLAGMASK(New && Updates.Results.NewsUrl.IsEmpty(), qaAll) |
-      FLAGMASK(New, qaCancel) |
-      FLAGMASK(!Updates.Results.Url.IsEmpty(), qaYes);
-    TQueryButtonAlias Aliases[4];
-    Aliases[0].Button = qaYes;
-    if (Updates.Results.UrlButton.IsEmpty())
-    {
-      Aliases[0].Alias = LoadStr(UPDATE_URL_BUTTON);
-    }
-    else
-    {
-      Aliases[0].Alias = Updates.Results.UrlButton;
-    }
-    Aliases[1].Button = qaAll;
-    Aliases[1].Alias = LoadStr(WHATS_NEW_BUTTON);
-    Aliases[1].OnSubmit = MakeMethod<TButtonSubmitEvent>(NULL, OpenHistory);
-    Aliases[2].Button = qaCancel;
-    Aliases[2].Alias = Vcl_Consts_SMsgDlgClose;
-    // Used only when New == true, see AliasesCount below
-    Aliases[3].Button = qaOK;
-    Aliases[3].Alias = LoadStr(UPGRADE_BUTTON);
-    if (!Updates.Results.DownloadUrl.IsEmpty())
-    {
-      Aliases[3].OnSubmit = MakeMethod<TButtonSubmitEvent>(NULL, DownloadUpdate);
-      Aliases[3].ElevationRequired = true;
-    }
+  TMessageParams Params;
+  Params.Aliases = Aliases;
+  Params.MoreMessagesUrl = Updates.Results.NewsUrl;
+  Params.MoreMessagesSize = Updates.Results.NewsSize;
+  // alias "ok" button to "upgrade" only if we have new version
+  Params.AliasesCount = LENOF(Aliases) - (New ? 0 : 1);
+  Params.CustomCaption = LoadStr(CHECK_FOR_UPDATES_TITLE);
 
-    TMessageParams Params;
-    Params.Aliases = Aliases;
-    Params.MoreMessagesUrl = Updates.Results.NewsUrl;
-    Params.MoreMessagesSize = Updates.Results.NewsSize;
-    // alias "ok" button to "upgrade" only if we have new version
-    Params.AliasesCount = LENOF(Aliases) - (New ? 0 : 1);
-    Params.CustomCaption = LoadStr(CHECK_FOR_UPDATES_TITLE);
+  if (New)
+  {
+    Params.ImageName = L"Installer";
+  }
+
+  std::unique_ptr<TForm> Dialog(
+    CreateMoreMessageDialogEx(Message, NULL, Type, Answers, HELP_UPDATES, &Params));
 
-    if (New)
+  if (New)
+  {
+    // Internet Explorer on Windows XP cannot talk to CDN77, where we host Store Get button.
+    // As a simple solution, we just do not display the donation panel on Windows XP.
+    if (Updates.Results.DownloadUrl.IsEmpty() && IsInstalled() && IsWinVista())
     {
-      Params.ImageName = L"Installer";
+      DebugAssert(Dialog->OnShow == NULL);
+      // InsertDonateLink need to be called only after MessageBrowser is created
+      Dialog->OnShow = MakeMethod<TNotifyEvent>(NULL, InsertDonateLink);
     }
+  }
 
-    std::unique_ptr<TForm> Dialog(
-      CreateMoreMessageDialogEx(Message, NULL, Type, Answers, HELP_UPDATES, &Params));
-
-    if (New)
-    {
-      // Internet Explorer on Windows XP cannot talk to CDN77, where we host Store Get button.
-      // As a simple solution, we just do not display the donation panel on Windows XP.
-      if (Updates.Results.DownloadUrl.IsEmpty() && IsInstalled() && IsWinVista())
+  unsigned int Answer = ExecuteMessageDialog(Dialog.get(), Answers, &Params);
+  switch (Answer)
+  {
+    case qaOK:
+      if (New)
       {
-        DebugAssert(Dialog->OnShow == NULL);
-        // InsertDonateLink need to be called only after MessageBrowser is created
-        Dialog->OnShow = MakeMethod<TNotifyEvent>(NULL, InsertDonateLink);
+        Configuration->Usage->Inc(L"UpdateDownloadOpens");
+        UnicodeString UpgradeUrl = ProgramUrl(LoadStr(UPGRADE_URL));
+        UpgradeUrl = WantBetaUrl(UpgradeUrl, true);
+        UpgradeUrl = AppendUrlParams(UpgradeUrl, FORMAT(L"to=%s", (VersionStrFromCompoundVersion(Updates.Results.Version))));
+        OpenBrowser(UpgradeUrl);
+        Result = true;
       }
-    }
-
-    unsigned int Answer = ExecuteMessageDialog(Dialog.get(), Answers, &Params);
-    switch (Answer)
-    {
-      case qaOK:
-        if (New)
-        {
-          Configuration->Usage->Inc(L"UpdateDownloadOpens");
-          UnicodeString UpgradeUrl = ProgramUrl(LoadStr(UPGRADE_URL));
-          UpgradeUrl = WantBetaUrl(UpgradeUrl, true);
-          UpgradeUrl = AppendUrlParams(UpgradeUrl, FORMAT(L"to=%s", (VersionStrFromCompoundVersion(Updates.Results.Version))));
-          OpenBrowser(UpgradeUrl);
-          Result = true;
-        }
-        break;
+      break;
 
-      case qaYes:
-        OpenBrowser(Updates.Results.Url);
-        break;
+    case qaYes:
+      OpenBrowser(Updates.Results.Url);
+      break;
 
-      case qaAll:
-        DebugFail();
-        break;
-    }
-  }
-  __finally
-  {
-    if (ActiveForm)
-    {
-      ActiveForm->Enabled = true;
-    }
+    case qaAll:
+      DebugFail();
+      break;
   }
   return Result;
 }