From b781b3e9fd1ff248294fe656e18c86050d65d46f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jonas=20Dre=C3=9Fler?= <verdre@v0yd.nl>
Date: Fri, 24 Jun 2022 10:38:44 +0200
Subject: [PATCH] appDisplay: Get linear index using item instead of page and
 position

_getLinearPosition() is a function that converts a page and position
index to the "accumulated" index that includes all pages before the
page. The function is used by _addItem() and _moveItem() for getting the
new index of an item inside the _orderedItems array.

Now when passing -1 as position to _addItem() or _moveItem(), this means
the item should be appended to the page. Right now _getLinearPosition()
returns the last item index on the page when passed -1, inserting the
item into _orderedItems at this index will actually not append it, but
insert it between the second last and last item.

To fix it, let's make the whole thing more robust by explicitly passing
an item to _getLinearPosition(). This means we simply no longer have to
assume what "-1" means. Moving the call to _getLinearPosition() to
happen after addItem() and moveItem() ensures that the new item position
is used and not the old one.

This fixes issues where the _orderedItems array gets out of order when
moving or adding items.

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2348>
---
 js/ui/appDisplay.js | 36 +++++++++++++-----------------------
 1 file changed, 13 insertions(+), 23 deletions(-)

diff --git a/js/ui/appDisplay.js b/js/ui/appDisplay.js
index abbadcbed..5e9ad63fe 100644
--- a/js/ui/appDisplay.js
+++ b/js/ui/appDisplay.js
@@ -1054,20 +1054,18 @@ var BaseAppView = GObject.registerClass({
         return -1;
     }
 
-    _getLinearPosition(page, position) {
+    _getLinearPosition(item) {
+        const [page, position] = this._grid.getItemPosition(item);
+        if (page === -1 || position === -1)
+            throw new Error('Item not found in grid');
+
         let itemIndex = 0;
 
         if (this._grid.nPages > 0) {
-            const realPage = page === -1 ? this._grid.nPages - 1 : page;
+            for (let i = 0; i < page; i++)
+                itemIndex += this._grid.getItemsAtPage(i).filter(c => c.visible).length;
 
-            itemIndex = position === -1
-                ? this._grid.getItemsAtPage(realPage).filter(c => c.visible).length - 1
-                : position;
-
-            for (let i = 0; i < realPage; i++) {
-                const pageItems = this._grid.getItemsAtPage(i).filter(c => c.visible);
-                itemIndex += pageItems.length;
-            }
+            itemIndex += position;
         }
 
         return itemIndex;
@@ -1079,11 +1077,10 @@ var BaseAppView = GObject.registerClass({
         if (this._grid.nPages > 1 && page === -1 && position === -1)
             page = this._findBestPageToAppend();
 
-        const itemIndex = this._getLinearPosition(page, position);
-
-        this._orderedItems.splice(itemIndex, 0, item);
         this._items.set(item.id, item);
         this._grid.addItem(item, page, position);
+
+        this._orderedItems.splice(this._getLinearPosition(item), 0, item);
     }
 
     _removeItem(item) {
@@ -1227,18 +1224,11 @@ var BaseAppView = GObject.registerClass({
     }
 
     _moveItem(item, newPage, newPosition) {
-        const [page, position] = this._grid.getItemPosition(item);
-        if (page === newPage && position === newPosition)
-            return;
+        this._grid.moveItem(item, newPage, newPosition);
 
         // Update the _orderedItems array
-        let index = this._orderedItems.indexOf(item);
-        this._orderedItems.splice(index, 1);
-
-        index = this._getLinearPosition(newPage, newPosition);
-        this._orderedItems.splice(index, 0, item);
-
-        this._grid.moveItem(item, newPage, newPosition);
+        this._orderedItems.splice(this._orderedItems.indexOf(item), 1);
+        this._orderedItems.splice(this._getLinearPosition(item), 0, item);
     }
 
     vfunc_map() {