Menu links for Petitions, Surveys do not open correct tab

Overview

The Campaigns menu has a Dashboard entry with three sub entries, 2 of which don't work (and one only works in the same way a stopped clock works twice a day)

Reproduction steps

Click Campaigns » Dashboards » Petition or Survey

Current behaviour

You just get the main campaigns page.

Expected behaviour

Should select the correct tab.

Environment information

• CiviCRM: 5.30, 5.31

added git:civicrm-core regression sig:bug triaged labels

@artfulrobot are you able to provide a PR?

And I have given this the regression label as I assume this was working correctly before. Feel free to remove the regression label if anyone thinks otherwise.

I remember @mattwire worked on something to do with tabs recently-ish but I was unable to find it in lab. I'm really snowed under at the mo. Generally, if I can fix something I'll do a PR, but when I can't I'll list it as an issue so it's acknowledged/might save someone else time.

Confirm it's not working. Seems to be trying to use subPage as a key in the URL instead of the "standard" selectedChild but that's not working either!

I'm investigating. Seems to be a Javascript problem...

@mattwire I've tracked the problem down to the file templates/CRM/common/TabSelected.tpl. I'm honestly not sure if that file is needed. It appears to be redundant.

What's supposed to happen is that jQuery tabs are initialized by templates/CRM/common/TabHeader.js which sets the "active" tab on lines 8-9:

  var tabSettings = CRM.tabSettings || {};
  tabSettings.active = tabSettings.active ? $('#tab_' + tabSettings.active).prevAll().length : 0;

The setting CRM.tabSettings.active is typically set by the page run() function, e.g. CRM_Campaign_Page_DashBoard does this:

  CRM_Core_Resources::singleton()
    ->addScriptFile('civicrm', 'templates/CRM/common/TabHeader.js', 1, 'html-header')
    ->addSetting([
      'tabSettings' => [
        'active' => strtolower(CRM_Utils_Array::value('subPage', $_GET, 'campaign')),
      ],
    ]);

Which fetches a param from the url and assigns it to the javascript variable, which in turn gets passed into the jQuery tabs() constructor on line 54 of templates/CRM/common/TabHeader.js.

So... I'm guessing there was some kind of misunderstanding about this which led you to write the file templates/CRM/common/TabSelected.tpl, which kind of does the same thing, but having two different scripts both attempting to set the default tab is bound to cause conflicts. Can we just remove that tpl in favor of the original approach?

@mattwire I've confirmed that removing that template and references to it will fix the problem. It also seems like a good cleanup to not be doing the same thing twice in two competing ways. But I think it will take some work to confirm that each page that uses tabs is still ok without it.

Meanwhile, a little preliminary cleanup: https://github.com/civicrm/civicrm-core/pull/19065

The tabs stuff has gone through a few rounds of cleanup / fixing and probably needs more. I have a vague memory that removing the TabSelected.tpl was a step too far last time I touched this code but be great to be able to remove it / simplify further. Have merged the cleanup.

closed with commit 5d219674

reopened

This fixes the bug and removes TabSelected.tpl from just about every screen in the system that uses tabs. I believe I've done it in a safe/compatible way but testing will be needed:

https://github.com/civicrm/civicrm-core/pull/19066

closed with commit 5d219674