Skip to content

Addition of the webextensions.api.tabGroups documentation #39370

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 22 commits into from
May 23, 2025

Conversation

rebloor
Copy link
Contributor

@rebloor rebloor commented May 2, 2025

Description

This PR adds the tabGroups API as documented in the API schema with additions for Chrome-only features as documented.

Related issues and pull requests

  • release note details documented in (PR TBD)
  • BCD details documented in (PR TBD)

@rebloor rebloor requested a review from a team as a code owner May 2, 2025 18:50
@rebloor rebloor requested review from willdurand and removed request for a team May 2, 2025 18:50
@github-actions github-actions bot added Content:WebExt WebExtensions docs size/l [PR only] 501-1000 LoC changed labels May 2, 2025
@rebloor rebloor marked this pull request as draft May 2, 2025 18:54
Copy link
Contributor

github-actions bot commented May 2, 2025

@rebloor rebloor requested review from dotproto and Rob--W and removed request for willdurand May 2, 2025 18:54
@rebloor rebloor self-assigned this May 2, 2025
@rebloor rebloor marked this pull request as ready for review May 5, 2025 04:13
Copy link
Member

@Rob--W Rob--W left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also update tabs group(), ungroup(), Tab type, query articles to refer to the tabGroups article.

Comment on lines 11 to 12

This event doesn't fire when a tab group is moved between windows; instead, the group is removed from one window and created in another (firing {{WebExtAPIRef("tabGroups.onRemoved")}} and {{WebExtAPIRef("tabGroups.onCreated")}}.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This event doesn't fire when a tab group is moved between windows; instead, the group is removed from one window and created in another (firing {{WebExtAPIRef("tabGroups.onRemoved")}} and {{WebExtAPIRef("tabGroups.onCreated")}}.

This text is copied from Chrome but not true in Firefox.

Relevant discussion in https://issues.chromium.org/issues/414993460#comment8 and the whole bug report in general.

rebloor and others added 9 commits May 8, 2025 14:49
…index.md

Co-authored-by: Rob Wu <rob@robwu.nl>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Rob Wu <rob@robwu.nl>
Co-authored-by: Rob Wu <rob@robwu.nl>
Co-authored-by: Rob Wu <rob@robwu.nl>
Co-authored-by: Rob Wu <rob@robwu.nl>
Co-authored-by: Rob Wu <rob@robwu.nl>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Fires when a tab group is removed. This can occur when a user closes a tab group or a tab group is closed automatically because another change means it no longer contained any tabs.

> [!NOTE] In Firefox, this event does not fire when a window is closed ([bug 1965007](https://bugzil.la/1965007)). As an alternative, use {{WebExtAPIRef("windows.onRemoved")}} to detect closed windows, and retrieve the remaining set of groups with {{WebExtAPIRef("tabGroups.query()")}}.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[markdownlint] reported by reviewdog 🐶
search-replace Custom rule [bad-gfm-alert: Use the correct GFM syntax: > [!NOTE]] [Context: "column: 1 text:'> [!NOTE]'"]

@rebloor rebloor requested a review from Rob--W May 8, 2025 15:19
Copy link
Member

@Rob--W Rob--W left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some clarifications, but mostly fixups (for missing periods in types and a few other smaller things).

I expect the PR to be ready to merge at the next review.

Co-authored-by: Rob Wu <rob@robwu.nl>
@rebloor rebloor requested a review from Rob--W May 12, 2025 02:35

This API enables extensions to modify and rearrange [tab groups](https://support.mozilla.org/en-US/kb/tab-groups).

Tab groups persist across browser restarts, except for tab groups in private browsing windows. When a tab group is restored, its tab group ID may differ from its original value.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this line use groupId to be more explicit? If we have an established pattern in cases like this, let's use that. If not, I personally prefer using field names over descriptive names in situations like this.

Suggested change
Tab groups persist across browser restarts, except for tab groups in private browsing windows. When a tab group is restored, its tab group ID may differ from its original value.
Tab groups persist across browser restarts, except for tab groups in private browsing windows. When a tab group is restored, its `groupId` may differ from its original value.

Copy link
Member

@Rob--W Rob--W left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I'll let you and @dotproto complete the rest of the review (with the points that he added).

Co-authored-by: Simeon Vincent <svincent@gmail.com>
Co-authored-by: Rob Wu <rob@robwu.nl>
@rebloor rebloor requested a review from dotproto May 12, 2025 11:52
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Comment on lines 11 to 13

> [!NOTE]
> In Firefox, this event doesn't fire when a window closes ([bug 1965007](https://bugzil.la/1965007)). As an alternative, use {{WebExtAPIRef("windows.onRemoved")}} to detect closed windows, and retrieve the remaining set of groups with {{WebExtAPIRef("tabGroups.query()")}}.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
> [!NOTE]
> In Firefox, this event doesn't fire when a window closes ([bug 1965007](https://bugzil.la/1965007)). As an alternative, use {{WebExtAPIRef("windows.onRemoved")}} to detect closed windows, and retrieve the remaining set of groups with {{WebExtAPIRef("tabGroups.query()")}}.

Suggested edit to remove the remark, because I plan to fix this issue and backport the patch so that the issue never reaches the 139 release.

- `moveProperties`
- : An object containing details of the location to move the tab group to.
- `index`
- : `integer`. The position to move the group to. Use -1 to place the group at the end of the window.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- : `integer`. The position to move the group to. Use -1 to place the group at the end of the window.
- : `integer`. The position to move the group to. After moving, the first tab in the tab group will be at this index in the tab strip. Use -1 to place the group at the end of the window. Groups cannot be moved before a pinned tab or inside another group.

Clarification, because "the position" is apparently ambiguous. Here is a bug report in Firefox that we plan to address before the 139 release: https://bugzilla.mozilla.org/show_bug.cgi?id=1963825

The last sentence of this, I'm not sure if it should be in index or generally at the top of the documentation.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the note about pinned tabs, I'd like to keep this line here, but I think it may also make sense to mention movement limitations in subsection (or part) of the overview.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


- `group`
- : {{WebExtAPIRef("tabGroups.TabGroup")}}. Details of the removed tab group's state.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also add removeInfo as additional information, with property isWindowClosing? You can re-use the content from https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/tabs/onRemoved , with just isWindowClosing (and no windowId), and "tab" replaced by "tab group".

This is part of the changes in https://bugzilla.mozilla.org/show_bug.cgi?id=1965007

Note: removeInfo is only supported in Firefox (planned for 139), Chrome does NOT have it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Rob--W done, I assume there was no need to document this as an extra object.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Rob--W done, I assume there was no need to document this as an extra object.

You wrote "done", but the change is not reflected in the PR yet.

If by "not an extra object", you meant "not a separate article", then my answer is yes.

The BCD does need a new PR for removeInfo, because Chrome does not support it.

@rebloor rebloor requested a review from Rob--W May 14, 2025 23:50
Copy link
Collaborator

@dotproto dotproto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this PR is very close. It looks like there are only a few items left

  • Two comment thread that have been marked "done" but haven't been pushed yet (thread 1, thread 2)
  • A note that can be removed (thread)

I took the liberty of resolving my example suggestion thread.

Once those threads are resolve I'm good to approve.

@rebloor
Copy link
Contributor Author

rebloor commented May 21, 2025

@dotproto missing commit pushed. Please check.

Copy link
Member

@Rob--W Rob--W left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to merge with these changes included. Please also update the BCD to mention removeInfo in tabGroups.onRenoved, it is not supported in Chrome, only in Firefox.

Comment on lines 11 to 13

> [!NOTE]
> In Firefox, this event doesn't fire when a window closes ([bug 1965007](https://bugzil.la/1965007)). As an alternative, use {{WebExtAPIRef("windows.onRemoved")}} to detect closed windows, and retrieve the remaining set of groups with {{WebExtAPIRef("tabGroups.query()")}}.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop note, we fixed this issue before release: https://bugzilla.mozilla.org/show_bug.cgi?id=1965007#a729217_640478

rebloor and others added 2 commits May 22, 2025 10:23
Co-authored-by: Rob Wu <rob@robwu.nl>
@Rob--W
Copy link
Member

Rob--W commented May 23, 2025

This PR cannot be merged because @dotproto is marked as a blocking reviewer. He however wrote that the PR looks good with minor adjustments (#39370 (review)), which I confirm have since been applied.

@rebloor
Copy link
Contributor Author

rebloor commented May 23, 2025

@Rob--W even when @dotproto has approved it still won't be possible for me to merge without "1 approving review by reviewers with write access." This appears to be a new requirement, and I followed up on Slack to get a list of appropriate reviewers.

@Rob--W
Copy link
Member

Rob--W commented May 23, 2025

@dotproto approved the other PR (in the BCD), not this one.

@rebloor rebloor merged commit 3ffce2e into mdn:main May 23, 2025
8 checks passed
@rebloor rebloor deleted the Add-tabGroups-API branch May 23, 2025 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebExt WebExtensions docs size/l [PR only] 501-1000 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants