Skip to content

2403: fix pinned tab new tab placement bug - #2470

Open
the-nelsonator wants to merge 1 commit into
mbnuqw:v5from
the-nelsonator:2403-new-tab-pos
Open

the-nelsonator wants to merge 1 commit into
mbnuqw:v5from
the-nelsonator:2403-new-tab-pos

Conversation

@the-nelsonator

Copy link
Copy Markdown
Contributor

Closes #2403

Problem

With a pinned tab active, a new tab created by the browser lands at the position of the general rule ("Place new tab (general rule)") instead of the position configured in its "If active tab is pinned" sub-option. Most visibly: middle-clicking a bookmark in the native bookmarks toolbar from a pinned tab opens the tab at panel end, even though both pinned-related settings default to panel start.

Root cause

In getIndexForNewTab() (src/services/tabs.fg.create.ts), the moveNewTabActivePin / moveNewTabButtonActivePin override was gated behind two conditions that don't hold in this scenario:

  1. It was only consulted inside the before / after / first_child / last_child branches. With the default general rule panel end, the function returns nextIndex before reaching any of them, so the override was never read. The setup page also greys the field out for start/end, while still showing its (default) value of panel start, so the setting looks like it should apply.
  2. Even in those branches it required activeTab.panelId === panel.id. For globally pinned tabs (pinnedTabsPosition: 'top' — now the default for new users) a pinned tab's panelId is just the last panel it happened to be associated with and carries no meaning. When it didn't match the target panel, if (!activeTab || activeTab.panelId !== panel.id) return nextIndex fired first and the tab went to panel end.

The other pinned setting, moveNewTabPin ("Place new tab opened from pinned tab", default start), is keyed on parent.pinned, and a tab middle-clicked from the bookmarks toolbar has no openerTabId (Firefox opens places links with relatedToCurrent: false, which is also why the tab appears at the end of the tab strip). So that rule is skipped as well and placement falls through to the general rule.

How to reproduce

A — deterministic, stock settings, single panel:

  1. Fresh profile, defaults (general rule = panel end, "If active tab is pinned" = panel start, greyed out).
  2. Pin a tab, keep at least one unpinned tab, activate the pinned tab.
  3. Open a tab the browser creates without an opener: middle-click a bookmark in the bookmarks toolbar, "Open in new tab" from the Library/History menu, or follow a link from an external application.

→ The tab lands at panel end. Sidebery's own entry points (new-tab button, bookmarks/history panels, middle-click inside the sidebar) can't reproduce it, because they pass an explicit destination or an opener.

B — the intermittent variant (needs >= 2 tabs panels and a relative general rule such as after), which explains why unpinning/repinning appears to fix it:

  1. Panels A and B, pinned tabs global, "If active tab is pinned" = panel start.
  2. The pinned tab's panelId is A; switch to panel B, activate the pinned tab, open a bookmark from the toolbar → it goes to B's end, override ignored.
  3. Unpin + repin that tab while B is active — the unpin handler stamps tab.panelId = <active panel> (tabs.fg.handlers.ts:1000) — and the override starts working for that tab.
  4. Restart Firefox, where restoreTab() reassigns every global pinned tab to the first tabs panel (tabs.fg.ts:533-535), or simply switch to another panel, and the misplacement returns.

Changes

  • src/services/tabs.fg.create.ts — lifted the pinned-active-tab check out of the four relative branches so it overrides every general rule except none ("do not move it"), and added isPinnedTabRelatedToPanel() so the panelId comparison is only applied when pinned tabs actually live in panels (pinnedTabsPosition === 'panel'). Covers both moveNewTabActivePin and moveNewTabButtonActivePin. The now-unreachable per-branch copies of the check were removed.
  • src/page.setup/components/settings.tabs.vue — the override field is no longer greyed out for start/end, since it now applies there; it is inactive only for none. newTabPosRelativeToActiveTab / newTabBtnPosRelativeToActiveTab became newTabPosDependsOnActiveTab / newTabBtnPosDependsOnActiveTab.
  • src/services/tabs.fg.create.test.ts (new) — 10 tests for getIndexForNewTab().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Bookmark bar new tabs from pinned tab don't respect "Place new tab (general rule) > If active tab is pinned" setting

1 participant