-
Notifications
You must be signed in to change notification settings - Fork 22.7k
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
Conversation
There was a problem hiding this 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.
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/oncreated/index.md
Outdated
Show resolved
Hide resolved
|
||
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")}}. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/onmoved/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/tabgroup/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/update/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabs/onupdated/index.md
Outdated
Show resolved
Hide resolved
…index.md 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> 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()")}}. |
There was a problem hiding this comment.
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]'"]
There was a problem hiding this 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.
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/move/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/oncreated/index.md
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/onmoved/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/onmoved/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/query/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/update/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/update/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/update/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/move/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/move/index.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Rob Wu <rob@robwu.nl>
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/color/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/tabgroup/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/onmoved/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/onremoved/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/onmoved/index.md
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/onmoved/index.md
Show resolved
Hide resolved
|
||
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. |
There was a problem hiding this comment.
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.
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. |
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/onmoved/index.md
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/onmoved/index.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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).
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/onmoved/index.md
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/tabgroup/index.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Simeon Vincent <svincent@gmail.com> Co-authored-by: Rob Wu <rob@robwu.nl>
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/onupdated/index.md
Outdated
Show resolved
Hide resolved
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/onupdated/index.md
Outdated
Show resolved
Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
||
> [!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()")}}. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
> [!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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- : `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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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. | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this 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.
@dotproto missing commit pushed. Please check. |
There was a problem hiding this 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.
|
||
> [!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()")}}. |
There was a problem hiding this comment.
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
files/en-us/mozilla/add-ons/webextensions/api/tabgroups/onremoved/index.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Rob Wu <rob@robwu.nl>
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. |
@dotproto approved the other PR (in the BCD), not this one. |
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