Bug 1824358 - Don't update textContent for elements in Recently Close…

…d tabs in Fx View to prevent Lit errors r=mstriemer,fxview-reviewers,sclements, a=dsmith commit 13912140b45ae09f71341b469c8ece26fcbe0885 Author: Kelly Cochrane <[email protected]> Date: Tue Mar 28 10:08:10 2023 -0400 Bug 1824358 - Don't update textContent for elements in Recently Closed tabs in Fx View to prevent Lit errors r=mstriemer,fxview-reviewers,sclements
dothq · Mar 28, 2023 · fe1b040 · fe1b040
1 parent 34b5628
commit fe1b040
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 24 deletions.
diff --git a/browser/components/firefoxview/recently-closed-tabs.mjs b/browser/components/firefoxview/recently-closed-tabs.mjs
@@ -48,7 +48,11 @@ class RecentlyClosedTabsList extends MozLitElement {
       "timeMsPref",
       "browser.tabs.firefox-view.updateTimeMs",
       NOW_THRESHOLD_MS,
-      () => this.updateTime()
+      timeMsPref => {
+        clearInterval(this.intervalID);
+        this.intervalID = setInterval(() => this.requestUpdate(), timeMsPref);
+        this.requestUpdate();
+      }
     );
   }
 
@@ -70,23 +74,13 @@ class RecentlyClosedTabsList extends MozLitElement {
 
   connectedCallback() {
     super.connectedCallback();
-    this.intervalID = setInterval(() => this.updateTime(), lazy.timeMsPref);
+    this.intervalID = setInterval(() => this.requestUpdate(), lazy.timeMsPref);
   }
 
   disconnectedCallback() {
     clearInterval(this.intervalID);
   }
 
-  updateTime() {
-    for (let timeEl of this.timeElements) {
-      timeEl.textContent = convertTimestamp(
-        parseInt(timeEl.getAttribute("data-timestamp")),
-        this.fluentStrings,
-        lazy.timeMsPref
-      );
-    }
-  }
-
   getTabStateValue(tab, key) {
     let value = "";
     const tabEntries = tab.state.entries;
@@ -210,13 +204,13 @@ class RecentlyClosedTabsList extends MozLitElement {
 
   updated() {
     let focusRestored = false;
-    if (this.lastFocusedIndex >= 0) {
-      if (this.tabsList && this.tabsList.children.length) {
+    if (
+      this.lastFocusedIndex >= 0 &&
+      (!this.tabsList || this.lastFocusedIndex >= this.tabsList.children.length)
+    ) {
+      if (this.tabsList) {
         let items = [...this.tabsList.children];
-        let newFocusIndex = Math.max(
-          Math.min(items.length - 1, this.lastFocusedIndex - 1),
-          0
-        );
+        let newFocusIndex = items.length - 1;
         let newFocus = items[newFocusIndex];
         if (newFocus) {
           focusRestored = true;
@@ -259,7 +253,11 @@ class RecentlyClosedTabsList extends MozLitElement {
 
   recentlyClosedTabTemplate(tab, primary) {
     const targetURI = this.getTabStateValue(tab, "url");
-    const convertedTime = convertTimestamp(tab.closedAt, this.fluentStrings);
+    const convertedTime = convertTimestamp(
+      tab.closedAt,
+      this.fluentStrings,
+      lazy.timeMsPref
+    );
     return html`
       <li
         class="closed-tab-li"

diff --git a/browser/components/firefoxview/tests/browser/browser_recently_closed_tabs.js b/browser/components/firefoxview/tests/browser/browser_recently_closed_tabs.js
@@ -338,10 +338,9 @@ add_task(async function test_time_updates_correctly() {
     },
     async browser => {
       const { document } = browser.contentWindow;
-      const numOfListItems = document.querySelector("ol.closed-tabs-list")
-        .children.length;
-      const lastListItem = document.querySelector("ol.closed-tabs-list")
-        .children[numOfListItems - 1];
+      const tabsList = document.querySelector("ol.closed-tabs-list");
+      const numOfListItems = tabsList.children.length;
+      const lastListItem = tabsList.children[numOfListItems - 1];
       const timeLabel = lastListItem.querySelector("span.closed-tab-li-time");
       let initialTimeText = timeLabel.textContent;
       Assert.stringContains(
@@ -654,6 +653,7 @@ add_task(async function test_reopen_recently_closed_tabs() {
  * dismissed by clicking on their respective dismiss buttons.
  */
 add_task(async function test_dismiss_tab() {
+  const TAB_UPDATE_TIME_MS = 5;
   Services.obs.notifyObservers(null, "browser:purge-session-history");
   Assert.equal(
     SessionStore.getClosedTabCount(window),
@@ -681,9 +681,34 @@ add_task(async function test_dismiss_tab() {
     await close_tab(tab1);
     await closedObjectsChanged();
 
+    await clearAllParentTelemetryEvents();
+
     const tabsList = document.querySelector("ol.closed-tabs-list");
+    const numOfListItems = tabsList.children.length;
+    const lastListItem = tabsList.children[numOfListItems - 1];
+    const timeLabel = lastListItem.querySelector("span.closed-tab-li-time");
+    let initialTimeText = timeLabel.textContent;
+    Assert.stringContains(
+      initialTimeText,
+      "Just now",
+      "recently-closed-tabs list item time is 'Just now'"
+    );
+
+    await SpecialPowers.pushPrefEnv({
+      set: [["browser.tabs.firefox-view.updateTimeMs", TAB_UPDATE_TIME_MS]],
+    });
 
-    await clearAllParentTelemetryEvents();
+    await BrowserTestUtils.waitForMutationCondition(
+      timeLabel,
+      { childList: true },
+      () => !timeLabel.textContent.includes("Just now")
+    );
+
+    isnot(
+      timeLabel.textContent,
+      initialTimeText,
+      "recently-closed-tabs list item time has updated"
+    );
 
     await dismiss_tab(tabsList.children[0], content);
 
@@ -785,6 +810,8 @@ add_task(async function test_dismiss_tab() {
       !document.querySelector("ol.closed-tabs-list"),
       "The recently closed tabs list is not displayed."
     );
+
+    await SpecialPowers.popPrefEnv();
   });
 });