The selector div.vertical-tabs div.vertical-tabs-panes fieldset.vertical-tabs-pane fieldset legend messes up modules like Metatag who want to use fieldsets inside of vertical tab panes.

I looked up the history of scheduler.css, and it looks like it was added five years ago in #1172040: Contrib solution for non-collapsible fieldsets and missing titles as a workaround for this core issue: #1015798: Fieldsets inside vertical tabs have no title and can't be collapsed. The core issue was fixed four years ago! It's time to remove it.

CommentFileSizeAuthor
#2 Selection_396.png67.78 KBDave Reid
#2 Selection_398.png73.92 KBDave Reid
#2 2715479-remove-vertical-tab-css.patch3.11 KBDave Reid
#4 2715479-remove-vertical-tab-css-1.1-do-not-test.patch3.01 KBDave Reid
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dave Reid created an issue. See original summary.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid as a volunteer and at Lullabot commented
Issue summary: View changes
Status: Active » Needs review
FileSize
Selection_396.png67.78 KB
Selection_398.png73.92 KB
2715479-remove-vertical-tab-css.patch3.11 KB

Current 7.x-1.x on the node form showing this is completely broken:

After patch showing things work just as expected without the CSS:

KeyboardCowboy’s picture

KeyboardCowboy
Primary language English
Location Denver, CO, USA
CreditAttribution: KeyboardCowboy as a volunteer and at Lullabot commented
Status: Needs review » Reviewed & tested by the community

Patch applies to current DEV branch and fixes issues. Looks good.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid as a volunteer and at Lullabot commented
FileSize
2715479-remove-vertical-tab-css-1.1-do-not-test.patch3.01 KB

Patch against 7.x-1.1 which we were using.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 4: 2715479-remove-vertical-tab-css-1.1-do-not-test.patch, failed testing.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid as a volunteer and at Lullabot commented
Status: Needs work » Reviewed & tested by the community

wtf, a do-not-test patch was not supposed to be tested.

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented
Related issues: +#1015798: Fieldsets inside vertical tabs have no title and can't be collapsed, +#1172040: Contrib solution for non-collapsible fieldsets and missing titles, +#2468819: Remove theming workarounds and delete scheduler.css

Hi Dave and KeyboardCowboy,
You are right, that @todo has been completely forgotten. Thanks for doing the historical investigation. What theme are you using which shows the messed-up titles? None of the core themes currently fail. Or is due to interaction with Metatag? (which I have not got, so have not tested with yet)

I have checked the core themes Seven, Bartik, Garland and Stark and they are all fine without the Scheduler workaround. I also checked it with contrib themes Mayo, Pixture Reloaded and Rubik and these are all fine. The scheduler.css code is still required for Tao, but given that this is only a base theme, on which others like Rubik are built, we are probably OK with that failing.

Patch against 7.x-1.1 which we were using

Is that a typo? If you are on Scheduler 7.x-1.1 maybe it would be good to upgrade to 1.4

wtf, a do-not-test patch was not supposed to be tested

Was it because the name had an extra '1' in the penultimate part? '2715479-remove-vertical-tab-css-1.1-do-not-test.patch' - maybe it has to be a clean '.do-not-test.'

We can definitely remove the .css file, but are you sure we also want to remove the jQuery addition of the theme name? 

-  var theme = Drupal.settings.ajaxPageState['theme'];
-  $("div.vertical-tabs").addClass(theme);

That addition should not be doing any harm, and some custom themes for scheduler-enabled sites may be using that selector.

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented
Status: Reviewed & tested by the community » Fixed

Patch against 7.x-1.1 which we were using

I can see from the patch that this was not a typo and the patch was against 1.1 - that version was released in April 2013, so you are more than three years behind. There have been lots of enhancements and new features in release 1.2, and 1.3 and 1.4 so you would benefit from upgrading :-)

I decided to leave the theme name class in the vertical tabs div, in case any custom themes are using it.
Thanks for your contributions. Committed and fixed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.