Activate tab by link anchor is broken

Able to reproduce the issue by using the steps in the IS.
Tried to apply patch #4 on drupal 10.1.x-dev with field_group 8.x-3.x
But the patch failed to apply.
See the error below:

Checking patch formatters/tabs/horizontal-tabs.js...
error: while searching for:

        var $this = $(this).addClass('horizontal-tabs-panes');
        var focusID = $(':hidden.horizontal-tabs-active-tab', this).val();
        var tab_focus;

        // Check if there are some details that can be converted to horizontal-tabs

error: patch failed: formatters/tabs/horizontal-tabs.js:30
error: formatters/tabs/horizontal-tabs.js: patch does not apply

Here is a rerolled version of the patch in #4 for the 3.4 version

I noticed a typo in that last patch. I don't know if it breaks in all instances, or just mine. When I fixed the typo I was able to use the /node/add/bill#edit-group-path in the 8.x-3.6.

if (hash !== '#' && $(hash, <strong>this</strong>).length) {
  tabFocus = $(hash, <strong>this</strong>).closest('.horizontal-tabs-pane');
} else {
  tabFocus = $this.find('> .horizontal-tabs-pane:first');
}bFocus = $this.find('> .horizontal-tabs-pane:first');
}

the two instances of this should be $this.

This typo is present in 8.x-3.6 and 3.x-dev

#7: According to jquery docs that should not matter.
https://api.jquery.com/jQuery/

I can confirm that #6 applies to current version and fixes the issue for me.

Conflicts need to be resolved, then we need RTBC again. Please use the MR.

Here is a new version of the patch from #6 for the 4.x version

I have rerolled the patch and added a test for each case.
However, I extended the MR to also include an update to the location hash. I don't see the point of having the ability to work when manually adding the fragment but not being able to see it when clicking the tabs.

Disclaimer though: Feel free to disagree and remove the last commit.

I confirm the issue is present in 4.x and the code in the MR fixes it.

Thank You, I can also confirm MR539983 works as expected.

Still needs tests ad there's an unresolved comment.

I see, also I have notice, that my web pages where "field group" is used is slower now. Does anyone experience the same issue after applied this patch?

Adding #12 patch will fix my issue to open correct tab by using anchor link.

With this patch:
Link /node/NID/edit#edit-group-workspace will open Workspace tab automatically

Without this patch:
Link /node/NID/edit#edit-group-workspace will open first tab Basic information automatically

Sorry, tests were added in #15, so this is ready for review!

I can't see reasons for possible performance issues (#19), but should be double-checked!

I didn't saw performance difference by adding patch but mainly I was focus my anchor link problem.

Claude Code says:

Summary

This branch adds the ability to activate a horizontal tab via a URL fragment (e.g. #edit-group-tab2), and updates the URL when users click between tabs. Two files are changed: the horizontal tabs JS and a new test.

---
JS Changes (formatters/tabs/horizontal-tabs.js)

1. Tab priority logic refactored (lines 63-135)

The original code used a single tabFocus variable and only checked the URL fragment as a fallback when no focusID matched. Now there are three variables — defaultTab, defaultTabFromLink, and tabFocus — separating the "saved active tab" from the "fragment-derived tab."

The final resolution at line 135 is:

tabFocus = defaultTabFromLink || defaultTab;

Issue: This inverts the priority. The URL fragment tab now always wins over the focusID (the hidden input that stores which tab was previously active, e.g. after a form submission with validation errors). But defaultTabFromLink is always set — when there's no matching fragment, it falls back to the first pane (line 132). So defaultTab (from focusID) can never be reached. The || defaultTab is dead code.

This means if a user submits a form, gets a validation error on Tab 2, and the page reloads without a fragment, Tab 1 will be shown instead of the tab containing the error. The test testRequiredFieldsActiveTab appears to test this scenario but may be passing because Drupal's separate validation-error-focusing JS (details_validation) opens the error tab after this code runs.

Recommendation: The fragment should take priority only when it actually matches a tab element, and focusID should be the fallback. Something like:

tabFocus = defaultTabFromLink || defaultTab || $this.find('> .horizontal-tabs-pane:first');

where defaultTabFromLink is only set when the hash actually matches (remove the else branch that assigns the first pane to it).

2. var instead of let (line 127)

var hash = window.location.hash.replace(/[=%;,\/]/g, '');

The rest of the file uses let/const. This should be const since hash is never reassigned.

3. history.replaceState on tab click (line 162)

history.replaceState(null, '', '#' + self.details.attr('id'));

Good addition — keeps the URL in sync when users click tabs. One minor note: this uses string concatenation while the href attribute two lines above uses a template literal. Inconsistent but functionally fine.

---
Test (HorizontalTabsActiveTabTest.php)

The test is well-structured with three test methods covering the main scenarios:

- testActiveTabByFragment — navigating with a fragment activates the correct tab
- testRequiredFieldsActiveTab — submitting with an empty required field activates the error tab
- testUrlUpdate — clicking tabs updates the URL fragment

Minor observations:

- Line 1, cspell comment duplicated — cspell:words horizontaltabbutton appears at line 3 and again as cspell:ignore horizontaltabbutton in the class docblock (line 14). Pick one; cspell:ignore in the docblock is sufficient.
- The test setup is thorough and follows existing patterns in the module (cf. HorizontalTabsLabelsTest).

---
Key Concern

The main functional issue is the tab priority logic. As written, defaultTabFromLink always has a value (either the fragment match or the first pane), making defaultTab unreachable. The fragment-based activation works, but it comes at the cost of the focusID mechanism, which is how Drupal preserves active tab state across form rebuilds (e.g. after AJAX or validation errors).

@benstallings maybe prepare a separate MR based on that to test for everyone?

And would be great to have functional tests for this anyway, so we ensure it won't break again.

@anybody I have made a new branch for that, but currently gitlab is telling me I'm not allowed to push to this project, even though I have push access to the fork. I'll try again later.

Nice work @benstallings and thanks for the test! Code LGTM so if we get another review here, I'm happy to merge this!

Did the review myself, LGTM. Thanks for the test!!