From a6852085c32cbaee7b10120ee5e730e0f32b0a38 Mon Sep 17 00:00:00 2001
From: trickypr <23250792+trickypr@users.noreply.github.com>
Date: Sat, 20 Jan 2024 19:14:13 +1100
Subject: [PATCH] =?UTF-8?q?=F0=9F=9A=B8=20User=20experience=20fixes=20(#52?=
 =?UTF-8?q?)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 apps/content/src/browser/browser.ts           |  1 +
 .../browser/components/omnibox/Omnibox.svelte | 19 +++++--
 apps/content/src/browser/lib/window/tab.ts    |  9 ++-
 apps/content/src/browser/lib/window/tabs.ts   | 55 +++++++++++--------
 .../shared/search/providers/EngineProvider.ts |  2 +
 .../shared/search/providers/URLProvider.ts    |  1 +
 .../shared/search/providers/EngineProvider.ts | 18 ++++++
 libs/link/types/globals/Elements.d.ts         |  2 +
 libs/shared/src/utilities/fn.ts               |  8 +++
 libs/shared/src/utilities/index.ts            |  2 +
 libs/shared/src/utilities/ring.ts             | 24 ++++++++
 11 files changed, 110 insertions(+), 31 deletions(-)
 create mode 100644 libs/shared/src/utilities/fn.ts
 create mode 100644 libs/shared/src/utilities/ring.ts

diff --git a/apps/content/src/browser/browser.ts b/apps/content/src/browser/browser.ts
index 49cd7d2..2550675 100644
--- a/apps/content/src/browser/browser.ts
+++ b/apps/content/src/browser/browser.ts
@@ -7,6 +7,7 @@ import 'remixicon/fonts/remixicon.css'
 import '@shared/styles/window.css'
 
 import App from './Browser.svelte'
+import './browser.css'
 import { initializeShortcuts } from './lib/shortcuts'
 import { initializeWindow } from './lib/window'
 import './lib/window/api'
diff --git a/apps/content/src/browser/components/omnibox/Omnibox.svelte b/apps/content/src/browser/components/omnibox/Omnibox.svelte
index 0d7897a..8cce5b6 100644
--- a/apps/content/src/browser/components/omnibox/Omnibox.svelte
+++ b/apps/content/src/browser/components/omnibox/Omnibox.svelte
@@ -20,6 +20,7 @@
   let inputElement: HTMLInputElement
   let suggestions: Suggestion[] = []
   let selectedSuggestion = 0
+  let selectionChange = false
 
   $: focusedOmnibox = tab.focusedOmnibox
   $: uri = tab.uri
@@ -73,13 +74,16 @@
         bind:value={inputContent}
         on:focusin={() => {
           focusedOmnibox.set(true)
+          selectionChange = false
           generateSuggestions()
         }}
-        on:blur|capture={() =>
-          setTimeout(() => {
-            focusedOmnibox.set(false)
-            suggestions = []
-          }, 100)}
+        on:blur|capture={() => setTimeout(() => focusedOmnibox.set(false), 100)}
+        on:mouseup={(_) => {
+          if (!selectionChange) inputElement.select()
+        }}
+        on:selectionchange={() => {
+          selectionChange = true
+        }}
         on:keyup={async (e) => {
           if (e.key === 'Enter') {
             focusedOmnibox.set(false)
@@ -119,8 +123,8 @@
 
     <div
       class="suggestions"
-      hidden={!$focusedOmnibox}
       id="omnibox__suggestions-list"
+      hidden={!$focusedOmnibox}
       role="listbox"
     >
       {#each suggestions as suggestion, index}
@@ -184,6 +188,9 @@
     padding: 0.5rem 1rem;
     border-radius: 0.25rem;
     cursor: pointer;
+    text-overflow: ellipsis;
+    white-space: nowrap;
+    overflow: hidden;
   }
 
   .suggestion:hover {
diff --git a/apps/content/src/browser/lib/window/tab.ts b/apps/content/src/browser/lib/window/tab.ts
index 8c9fe78..8906a8a 100644
--- a/apps/content/src/browser/lib/window/tab.ts
+++ b/apps/content/src/browser/lib/window/tab.ts
@@ -167,7 +167,11 @@ export class Tab {
     this.progressListener.events.on('locationChange', (event) => {
       if (!event.aWebProgress.isTopLevel) return
 
-      this.icon.set(null)
+      const sameLocation =
+        event.aFlags & 0x01 /* LOCATION_CHANGE_SAME_DOCUMENT */
+      if (!sameLocation) {
+        this.icon.set(null)
+      }
 
       this.uri.set(event.aLocation)
       this.canGoBack.set(this.browserElement.canGoBack)
@@ -219,8 +223,7 @@ export class Tab {
 
     const findbar = this.findbar.readOnce()
     if (findbar) {
-      if (findbar.hidden) findbar.open()
-      else findbar.close()
+      findbar.onFindCommand()
       return
     }
 
diff --git a/apps/content/src/browser/lib/window/tabs.ts b/apps/content/src/browser/lib/window/tabs.ts
index 1e82ee5..de8b9d7 100644
--- a/apps/content/src/browser/lib/window/tabs.ts
+++ b/apps/content/src/browser/lib/window/tabs.ts
@@ -1,15 +1,14 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-import { viewableWritable } from '@experiment/shared'
-import { writable } from 'svelte/store'
+import { Ring, not, viewableWritable } from '@experiment/shared'
 
 import { resource } from '../resources'
 import { Tab } from './tab'
 
-let internalSelectedTab = -1
-export const selectedTabId = writable(-1)
-selectedTabId.subscribe((v) => (internalSelectedTab = v))
+const tabHistory = new Ring<number>(10)
+export const selectedTabId = viewableWritable(-1)
+selectedTabId.subscribe((id) => tabHistory.next(id))
 
 const uriPref = (pref: string) => (): nsIURIType =>
   resource.NetUtil.newURI(Services.prefs.getStringPref(pref, 'about:blank'))
@@ -18,8 +17,16 @@ export const ABOUT_BLANK = resource.NetUtil.newURI('about:blank')
 
 export const tabs = viewableWritable<Tab[]>([])
 
+const matchTab = (id: number) => (tab: Tab) => tab.getId() === id
+
 export function openTab(uri: nsIURIType = newTabUri()) {
   const newTab = new Tab(uri)
+
+  // We only want to focus the omnibox on new tab pages
+  if (uri.asciiSpec != newTabUri().asciiSpec) {
+    newTab.focusedOmnibox.set(false)
+  }
+
   tabs.update((tabs) => {
     selectedTabId.set(newTab.getId())
     return [...tabs, newTab]
@@ -29,18 +36,24 @@ export function openTab(uri: nsIURIType = newTabUri()) {
 
 export function closeTab(tab: Tab) {
   tabs.update((tabs) => {
-    const tabIndex = tabs.findIndex((t) => t.getId() == tab.getId())
-    const filtered = tabs.filter((t) => t.getId() != tab.getId())
+    const tabIndex = tabs.findIndex(matchTab(tab.getId()))
+    const filtered = tabs.filter(not(matchTab(tab.getId())))
 
     if (filtered.length == 0) {
       window.close()
       return []
     }
 
-    if (filtered[tabIndex]) {
-      selectedTabId.set(filtered[tabIndex].getId())
+    const lastTabId = tabHistory.prev()
+    const hasLastTab = filtered.some(matchTab(lastTabId))
+    if (hasLastTab) {
+      selectedTabId.set(lastTabId)
     } else {
-      selectedTabId.set(filtered[tabIndex - 1].getId())
+      if (filtered[tabIndex]) {
+        selectedTabId.set(filtered[tabIndex].getId())
+      } else {
+        selectedTabId.set(filtered[tabIndex - 1].getId())
+      }
     }
 
     tab.destroy()
@@ -49,15 +62,15 @@ export function closeTab(tab: Tab) {
 }
 
 export function getTabById(id: number): Tab | undefined {
-  return tabs.readOnce().find((tab) => tab.getId() == id)
+  return tabs.readOnce().find(matchTab(id))
 }
 
 export function getCurrentTab(): Tab | undefined {
-  return getTabById(internalSelectedTab)
+  return getTabById(selectedTabId.readOnce())
 }
 
 export function setCurrentTab(tab: Tab) {
-  const index = tabs.readOnce().findIndex((t) => t.getId() == tab.getId())
+  const index = tabs.readOnce().findIndex(matchTab(tab.getId()))
   setCurrentTabIndex(index)
 }
 
@@ -67,7 +80,7 @@ export function runOnCurrentTab<R>(method: (tab: Tab) => R): R | undefined {
 }
 
 export function getCurrentTabIndex(): number {
-  return tabs.readOnce().findIndex((tab) => tab.getId() == internalSelectedTab)
+  return tabs.readOnce().findIndex(matchTab(selectedTabId.readOnce()))
 }
 
 export function setCurrentTabIndex(index: number) {
@@ -77,16 +90,14 @@ export function setCurrentTabIndex(index: number) {
   if (index < 0) index = allTabs.length - 1
   if (index >= allTabs.length) index = 0
 
-  selectedTabId.set(allTabs[index].getId())
+  const tabId = allTabs[index].getId()
+  selectedTabId.set(tabId)
 }
 
 export function moveTabBefore(toMoveId: number, targetId: number) {
   tabs.update((tabs) => {
-    const toMoveIndex = tabs.findIndex((tab) => tab.getId() == toMoveId)
-    const targetIndex = Math.max(
-      tabs.findIndex((tab) => tab.getId() == targetId) - 1,
-      0,
-    )
+    const toMoveIndex = tabs.findIndex(matchTab(toMoveId))
+    const targetIndex = Math.max(tabs.findIndex(matchTab(targetId)) - 1, 0)
 
     // If we do in-place modifications with tabs, svelte won't notice the
     // change
@@ -98,8 +109,8 @@ export function moveTabBefore(toMoveId: number, targetId: number) {
 
 export function moveTabAfter(toMoveId: number, targetId: number) {
   tabs.update((tabs) => {
-    const toMoveIndex = tabs.findIndex((tab) => tab.getId() == toMoveId)
-    const targetIndex = tabs.findIndex((tab) => tab.getId() == targetId)
+    const toMoveIndex = tabs.findIndex(matchTab(toMoveId))
+    const targetIndex = tabs.findIndex(matchTab(targetId))
 
     // If we do in-place modifications with tabs, svelte won't notice the
     // change
diff --git a/apps/content/src/shared/search/providers/EngineProvider.ts b/apps/content/src/shared/search/providers/EngineProvider.ts
index 28770a0..a493a4a 100644
--- a/apps/content/src/shared/search/providers/EngineProvider.ts
+++ b/apps/content/src/shared/search/providers/EngineProvider.ts
@@ -30,6 +30,8 @@ export class EngineProvider extends Provider {
   }
 
   async getFastResults(query: string) {
+    if (query == '' || isUrlLike(query)) return []
+    
     const engine = await this.engine.promise
     const defaultSearchResult: ProviderResult = {
       title: query,
diff --git a/apps/content/src/shared/search/providers/URLProvider.ts b/apps/content/src/shared/search/providers/URLProvider.ts
index 436cd4a..8e37982 100644
--- a/apps/content/src/shared/search/providers/URLProvider.ts
+++ b/apps/content/src/shared/search/providers/URLProvider.ts
@@ -39,6 +39,7 @@ export function isUrlLike(query: string): boolean {
 export class URLProvider extends Provider {
   public providerPriority = 0
 
+  /* eslint @typescript-eslint/no-unused-vars: 0 */
   public async getResults(_query: string) {
     return []
   }
diff --git a/apps/content/src/tests/shared/search/providers/EngineProvider.ts b/apps/content/src/tests/shared/search/providers/EngineProvider.ts
index c76a138..c383b24 100644
--- a/apps/content/src/tests/shared/search/providers/EngineProvider.ts
+++ b/apps/content/src/tests/shared/search/providers/EngineProvider.ts
@@ -57,6 +57,24 @@ export default async function () {
     )
   })
 
+  await test('EngineProvider: fast recomendations ignore URL like', async (t) => {
+    const provider = new EngineProvider()
+    {
+      const results = await provider.getFastResults('http://google.com')
+      t.eq(results, [], 'Should ignore full URLs')
+    }
+    {
+      const results = await provider.getFastResults('google.com')
+      t.eq(results, [], 'Should ignore domains')
+    }
+    {
+      const results = await provider.getFastResults(
+        'chrome://browser/content/tests/index.html',
+      )
+      t.eq(results, [], 'Should ignore chrome URLs')
+    }
+  })
+
   for (const engine of SEARCH_ENGINE_IDS) {
     await testProvider(engine)
   }
diff --git a/libs/link/types/globals/Elements.d.ts b/libs/link/types/globals/Elements.d.ts
index 8d52fe4..f68c010 100644
--- a/libs/link/types/globals/Elements.d.ts
+++ b/libs/link/types/globals/Elements.d.ts
@@ -68,6 +68,8 @@ declare interface XULFindBarElement extends HTMLElement {
   browser: XULBrowserElement
   open()
   close()
+
+  onFindCommand(): void
 }
 
 declare interface XULPanel extends HTMLElement {
diff --git a/libs/shared/src/utilities/fn.ts b/libs/shared/src/utilities/fn.ts
new file mode 100644
index 0000000..0700602
--- /dev/null
+++ b/libs/shared/src/utilities/fn.ts
@@ -0,0 +1,8 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+export const not =
+  <T extends Array<unknown>>(fn: (...args: T) => boolean) =>
+  (...args: T) =>
+    !fn(...args)
diff --git a/libs/shared/src/utilities/index.ts b/libs/shared/src/utilities/index.ts
index 2ec703e..0303dbc 100644
--- a/libs/shared/src/utilities/index.ts
+++ b/libs/shared/src/utilities/index.ts
@@ -3,5 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 export * from './deferred.js'
+export * from './fn.js'
 export * from './mitt.js'
+export * from './ring.js'
 export * from './svelte.js'
diff --git a/libs/shared/src/utilities/ring.ts b/libs/shared/src/utilities/ring.ts
new file mode 100644
index 0000000..957c4ae
--- /dev/null
+++ b/libs/shared/src/utilities/ring.ts
@@ -0,0 +1,24 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+export class Ring<T> {
+  private values: T[]
+  private index = -1
+
+  constructor(count = 5) {
+    this.values = Array(count)
+  }
+
+  public next(value: T) {
+    this.index += 1
+    if (this.index >= this.values.length) this.index = 0
+    this.values[this.index] = value
+  }
+
+  public prev(): T {
+    this.index -= 1
+    if (this.index <= -1) this.index = this.values.length - 1
+    return this.values[this.index]
+  }
+}