Bug 1438: When a passive file panel is focused after reload, the panel is scrolled to show the previously focused file, what can cause unintended behavior

https://winscp.net/tracker/1438

Source commit: 580a8d10cd990d0c0d33e65bcbc60c6167a15956

source/components/UnixDirView.cpp

@@ -561,7 +561,7 @@ void __fastcall TUnixDirView::DoReadDirectoryImpl(TObject * /*Sender*/, bool Rel
     }
     else
     {
-      Load();
+      Load(true);
       PathChanged();
     }
 
@@ -575,7 +575,7 @@ void __fastcall TUnixDirView::DoReadDirectoryImpl(TObject * /*Sender*/, bool Rel
     // Make sure file list is cleared, to remove all references to invalid
     // file objects. LoadFiles check for disconnected terminal, so no reloading
     // actually occures.
-    Load();
+    Load(true);
   }
 #else
   DebugUsedParam(ReloadOnly);

source/packages/filemng/CustomDirView.pas

@@ -326,12 +326,12 @@ type
     procedure DoDisplayPropertiesMenu;
     procedure DoExecute(Item: TListItem);
     procedure DoExecuteParentDirectory;
+    procedure Load(DoFocusSomething: Boolean); virtual;
     property ImageList16: TImageList read FImageList16;
     property ImageList32: TImageList read FImageList32;
   public
     constructor Create(AOwner: TComponent); override;
     destructor Destroy; override;
-    procedure Load; virtual;
     procedure Reload(CacheIcons: Boolean); virtual;
     function CreateFocusedFileList(FullPath: Boolean; FileList: TStrings = nil): TStrings;
     function CreateFileList(Focused: Boolean; FullPath: Boolean; FileList: TStrings = nil): TStrings;
@@ -1795,7 +1795,7 @@ begin
         end;
       end;
 
-      Load;
+      Load(False);
 
       OldSelection.Sort;
       if CacheIcons then IconCache.Sort;
@@ -1841,8 +1841,9 @@ begin
       FocusItem(ItemToFocus);
     end;
 
-    // cannot scroll when focus is not visible because
-    // of hack-implementation of FocusItem()
+    // could not scroll when focus is not visible because
+    // of previous hack-implementation of FocusItem()
+    // - no longer true, this can be re-enabled after some testing
     {$IF False}
     // previously focus item was not visible, scroll to the same position
     // as before
@@ -1864,7 +1865,7 @@ begin
   end;
 end;
 
-procedure TCustomDirView.Load;
+procedure TCustomDirView.Load(DoFocusSomething: Boolean);
 var
   SaveCursor: TCursor;
   Delimiters: string;
@@ -1943,7 +1944,10 @@ begin
 
         FNotifyEnabled := True;
 
-        FocusSomething;
+        if DoFocusSomething then
+        begin
+          FocusSomething;
+        end;
 
         if Assigned(FOnLoaded) then
         begin

source/packages/filemng/DirView.pas

@@ -237,6 +237,7 @@ type
     procedure StartFileDeleteThread;
     procedure WMDestroy(var Msg: TWMDestroy); message WM_DESTROY;
     procedure CMRecreateWnd(var Message: TMessage); message CM_RECREATEWND;
+    procedure Load(DoFocusSomething: Boolean); override;
     function HiddenCount: Integer; override;
     function FilteredCount: Integer; override;
 
@@ -300,7 +301,6 @@ type
     function GetFileRec(Index: Integer): PFileRec;
 
     {Populate / repopulate the filelist:}
-    procedure Load; override;
     procedure Reload(CacheIcons : Boolean); override;
     procedure Reload2;
 
@@ -805,7 +805,7 @@ begin
       SetLength(Value, Length(Value) - 1);
     PathChanging(True);
     FPath := Value;
-    Load;
+    Load(True);
   finally
     PathChanged;
   end;
@@ -1163,7 +1163,7 @@ begin
   end;
 end;
 
-procedure TDirView.Load;
+procedure TDirView.Load(DoFocusSomething: Boolean);
 begin
   try
     StopIconUpdateThread;
@@ -2813,7 +2813,7 @@ begin
   try
     PathChanging(False);
     FPath := ExtractFileDrive(Path);
-    Load;
+    Load(True);
   finally
     PathChanged;
   end;

source/packages/my/NortonLikeListView.pas

@@ -35,7 +35,6 @@ type
     FLButtonDownShiftState: TShiftState;
     FLButtonDownPos: TPoint;
     FLastSelectMethod: TSelectMethod;
-    FPendingInternalFocus: TListItem;
     procedure WMLButtonDown(var Message: TWMLButtonDown); message WM_LBUTTONDOWN;
     procedure WMRButtonDown(var Message: TWMRButtonDown); message WM_RBUTTONDOWN;
     procedure WMLButtonUp(var Message: TWMLButtonUp); message WM_LBUTTONUP;
@@ -80,7 +79,6 @@ type
     function GetSelCount: Integer; override;
     procedure DDBeforeDrag;
     function CanEdit(Item: TListItem): Boolean; override;
-    procedure DoEnter; override;
   public
     { Public declarations }
     constructor Create(AOwner: TComponent); override;
@@ -126,7 +124,6 @@ begin
   FUpdatingSelection := 0;
   FFocusingItem := False;
   FLastSelectMethod := smNoneYet;
-  FPendingInternalFocus := nil;
   // On Windows Vista, native GetNextItem for selection stops working once we
   // disallow deselecting any item (see ExCanChange).
   // So we need to manage selection state ourselves
@@ -223,10 +220,6 @@ begin
   begin
     ItemUnselected(Item, -1);
   end;
-  if FPendingInternalFocus = Item then
-  begin
-    FPendingInternalFocus := nil;
-  end;
   FLastDeletedItem := Item;
   inherited;
   FLastDeletedItem := nil;
@@ -605,71 +598,16 @@ begin
 end;
 
 procedure TCustomNortonLikeListView.FocusItem(Item: TListItem);
-var
-  P: TPoint;
-  PLastSelectMethod: TSelectMethod;
-  PDontUnSelectItem: Boolean;
-  PDontSelectItem: Boolean;
-  WParam: UINT_PTR;
-  LParam: INT_PTR;
-begin
-  // This whole is replacement for mere ItemFocused := Item
-  // because that does not reset some internal focused pointer,
-  // causing subsequent Shift-Click selects range from the first item,
-  // not from focused item.
-  Item.MakeVisible(False);
-  if Focused then
-  begin
-    P := Item.GetPosition;
-    PLastSelectMethod := FLastSelectMethod;
-    PDontSelectItem := FDontSelectItem;
-    PDontUnSelectItem := FDontUnSelectItem;
-    FLastSelectMethod := smNoneYet;
-    FDontSelectItem := True;
-    FDontUnSelectItem := True;
-    FFocusingItem := True;
-    try
-      // HACK
-      // WM_LBUTTONDOWN enters loop, waiting for WM_LBUTTONUP,
-      // so we have to post it in advance to break the loop immediately
-
-      // Without MK_CONTROL, if there are more items selected,
-      // they won't get unselected on subsequent focus change
-      // (with explorer-style selection).
-      // And it also makes the click the least obtrusive, affecting the focused
-      // file only.
-      WParam := MK_LBUTTON or MK_CONTROL;
-      LParam := MAKELPARAM(P.X, P.Y);
-      PostMessage(Handle, WM_LBUTTONUP, WParam, LParam);
-      SendMessage(Handle, WM_LBUTTONDOWN, WParam, LParam);
-    finally
-      FFocusingItem := False;
-      FLastSelectMethod := PLastSelectMethod;
-      FDontSelectItem := PDontSelectItem;
-      FDontUnSelectItem := PDontUnSelectItem;
-    end;
-    FPendingInternalFocus := nil;
-  end
-    else
-  begin
-    FPendingInternalFocus := Item;
-  end;
-  if ItemFocused <> Item then
-    ItemFocused := Item;
-end;
-
-procedure TCustomNortonLikeListView.DoEnter;
 begin
-  inherited;
-
-  if Assigned(FPendingInternalFocus) then
-  begin
-    if FPendingInternalFocus = ItemFocused then
-    begin
-      FocusItem(FPendingInternalFocus);
-    end;
-    FPendingInternalFocus := nil;
-  end;
+  // This method was introduced in 3.7.6 for reasons long forgotten.
+  // It simulated a mouse click on the item. Possibly because the mere ItemFocused := Item
+  // did not work due to at-the-time-not-realized conflict with FocusSomething.
+  // Now that this is fixed, the method is no longer needed.
+  // Keeping it for a while with this comment, in case some regression is discovered.
+  // References:
+  // - 3.7.6: When reloading directory content, file panel tries to preserve its position.
+  // - Bugs 999 and 1161
+  ItemFocused := Item;
 end;
 
 procedure TCustomNortonLikeListView.SelectAll(Mode: TSelectMode; Exclude: TListItem);