Skip to content

Commit

Permalink
Bug 1824358 - Don't update textContent for elements in Recently Close…
Browse files Browse the repository at this point in the history
…d tabs in Fx View to prevent Lit errors r=mstriemer,fxview-reviewers,sclements

Differential Revision: https://phabricator.services.mozilla.com/D173547
  • Loading branch information
kcochrane-mozilla committed Mar 27, 2023
1 parent 38a4785 commit 84eb43d
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 30 deletions.
40 changes: 17 additions & 23 deletions browser/components/firefoxview/recently-closed-tabs.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
);
}

Expand All @@ -70,27 +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() {
// when pref is 0, avoid the update altogether (used for tests)
if (!lazy.timeMsPref) {
return;
}
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;
Expand Down Expand Up @@ -214,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;
Expand Down Expand Up @@ -263,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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ async function dismiss_tab(tab, content) {
add_setup(async function setup() {
// set updateTimeMs to 0 to prevent unexpected/unrelated DOM mutations during testing
await SpecialPowers.pushPrefEnv({
set: [["browser.tabs.firefox-view.updateTimeMs", 0]],
set: [["browser.tabs.firefox-view.updateTimeMs", 100000]],
});
});

Expand Down Expand Up @@ -339,11 +339,10 @@ add_task(async function test_time_updates_correctly() {

await withFirefoxView({}, 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(
Expand Down Expand Up @@ -655,6 +654,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),
Expand Down Expand Up @@ -682,9 +682,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 clearAllParentTelemetryEvents();
await SpecialPowers.pushPrefEnv({
set: [["browser.tabs.firefox-view.updateTimeMs", TAB_UPDATE_TIME_MS]],
});

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);

Expand Down Expand Up @@ -786,6 +811,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();
});
});

Expand Down

0 comments on commit 84eb43d

Please sign in to comment.