Regression: Vertical Tabs styling on input elements overrides specifically set FAPI keys

I assume you realize why that was committed. Obviously, we didn't mean to break prefixes and suffixes, and will fix it. However, having fields extend beyond the container in almost every theme, in all browsers is just as big a WTF as far as I'm concerned.

I ran into a similar issue with yesterday, and came up with a better fix that should solve this problem for most browsers, but the ridiculous default size of 60 on the following fields is the source of the real problem here:

• file
• password
• password_confirm
• textarea
• textfield

60 is just too big for most situations (especially mobile and vertical tabs) and forces this kind of CSS to be written. IMO this should be fixed at the source so we don't need to provide CSS to fix a broken set of defaults and we can all be happy.

I ran into an issue doing a mobile theme w/password fields (not in vertical tabs, but same deal). I fixed it by removing width: 100% and using max-width: 100%.

I haven't fully tested, since it was just last night that I ran into it, but I'm pretty sure it will solve the problem perfectly in IE7+. I need to do a little more testing on it (I'd like to setup a test for #suffix/#prefix and a few other combos) just to make sure.

We still would need to deal with IE6 though... It might be worth adding a container class to theme_form_element() to deal with it if the FAPI size change wont be allowed?

Err, sorry I'm tired... That didn't make much sense. LOL. Let me try that again.

I ran into the problem having to special case password fields after applying the code in the original post to textfield and textareas. I applied it to all the fields because the problem actually exists on all of the fields (and bad) for mobile. The password fields couldn't be 100% because of the checker, so I fixed it as I noted above (max-width: 100%), and since it worked so well I replaced the original code with it and everything looks a gazillion times better with limited testing so far.

Great, I'm glad it works. Thanks for testing it and rolling the patch.

There are a couple of minor coding style issues:

+++ misc/vertical-tabs.css	13 Nov 2010 07:49:11 -0000
@@ -62,16 +62,20 @@ div.vertical-tabs {
+ * Prevent text inputs from overflowing when container is too narrow. ¶

Trailing white space.

+++ misc/vertical-tabs.css	13 Nov 2010 07:49:11 -0000
@@ -62,16 +62,20 @@ div.vertical-tabs {
+ * in conjunction with "box-sizing" to prevent box model issues from occurring in

The word "in" extends past 80 columns.

Powered by Dreditor.

I would like to fix the suffix/prefix issue out of the box in IE6. I think it would be a shame to make every theme that needs to support IE6 "deal" with this. I still think the FAPI default size of these elements should change, but yeah, the probability of that actually happening is really low at this point. I'll see if I can come up with anything that isn't too invasive in the meantime.

Ugh, well I did some more thorough testing, and this reverts fixes made in the previous patch. Apparently box-sizing needs an explicit width, and min/max-width alone don't work with it.

The only other thing I can think of to do (EDIT: with CSS), short of getting the default size changed is the following crazy hack that obviously will not fix all circumstances (i.e, when someone sets the size to something other than 60, that is too big for the container):

.vertical-tabs .form-type-textfield input[size="60"] {
  width: 100%;
  -moz-box-sizing: border-box;
  -webkit-box-sizing: border-box;
  box-sizing: border-box;
}

Screenshots of what I saw are attached.

Also, I can't figure out a good way to handle this in IE6. I was thinking we could add a class to the form element container based on:

1. whether a #field_suffix/prefix has been set.
2. whether a #size greater than 30 has been set.

Aside from the really ridiculous CSS that would mean adding, the problem is what do we set the CSS width to? Bartik breaks with two sidebars on size 27, Garland 36, etc. I guess we could tweak width of the tabs, to make Bartik work at 30, but the point is that it would still be a guess at the end of the day because we have no idea wide the suffix and prefix will be.

Specifically targeting the size=60 with CSS right now, and leaving IE6 broken is probably the best thing I can think of. It's not perfect, but it fixes both issues in IE7+, and overall it's not a very common occurrence that developers mess with size, other than to make an input smaller, from my experience.

I'm curious what you think about that or if you have any other ideas.

While I truly appreciate the work on this issue it's nowhere near critical. Vertical tabs did not even exist in Drupal 6 therefore it can not be a regression. Also, how would be so many users affected that we can't release? Compare this to data loss bugs, WSOD etc. You can always override a CSS if it really does not work for you.

Upon discussing the issue, it's major.

Here's the problem: Form API lets you define a #size which sets the visible size of an input element. However, setting width in CSS also does the same and has priority. Also, by taking all space possible, #field_prefix and #field_suffix no longer have the space to be on the left and right of the input element.

Agreed it looks a serious problem but -- only inside a vertical tab.

This is definitely not critical, but I agree that it's major.

Shipping with broken vertical tabs in all browsers for every theme that decides to use 2 sidebars is not an option. I'm sorry, but no. Hard coding pixel widths is not an option either. To impose this on all themes because the correct solution is too hard, or simply an API change late in the game for module developers is not something I am willing to sit here and watch.

If we really care about fixing this, then let's fix what's actually broken, instead of dumping it and blaming it on the theme layer. As mentioned in #1 the default size of 60, applied to certain field types is the problem. If we are willing to fix that great.

If not, I do not support reverting the previous patch.

Great!

This does solve the CSS problems, which is awesome. Some fields, like node titles look a little odd though. I think it would make sense to set those fields to 60 where it makes sense.

I'm gonna take inventory of these and report back soon.

subscribe.

If this patch affect Bartik, be sure to fix whatever gets borked, please.

Yeah, sorry. There's no way we can change the system defaults. Those have been in place since Drupal 4.7, literally since just after the FAPI was invented, and changing those would have tremendous effect on both module and theme developers.

But one-offing a #size on an affected field being displayed in vertical tabs seems fine.

That works for me. I think the defaults are something worth addressing, but 8.x seems more appropriate given that lots of fields use the 60 default, and the change would be pretty invasive to say the least.

Would it be a crazy to automatically adjust the size to a 30 max in template_preprocess_vertical_tabs() or a pre-render function?

This code, slightly changed, is still needed:

* html .vertical-tabs .form-type-textarea {
  width: 95%; /* IE6 */
}

Other than that, the patch is okay for all of the other themes are good, except Bartik and I've been having a hell of a time trying to make it behave because:

• top and border-radius being applied to the fieldsets which is being inherited.
• there's horizontal padding in .fieldset-wrapper in addition to what vertical tabs is providing, which makes the already small horizontal space even smaller.
• word-wrap: break-word, which took forever to track down is actually breaking the layout in IE6.
• The font size is just too big in combination with the 15em width of the tab list and the 960px.

The attached patch adds to Mark's patch in #26 and does the following:

in vertical-tabs.css:

• Makes the width of the tab list 13em instead of 15em. 15em is too wide with a 960px layout and two filled sidebars where it looks more like 50% tabs/50% content, and this is common. This change will not affect Seven because it uses a fixed width and doesn't have sidebar combinations to deal with, so I think it's fine.
• Sets .fieldset-wrapper padding to 0. It's pretty common to add padding to these and it's problematic inside vertical tabs, so vertical-tabs.css should try to prevent it.

in bartik's style.css

• Removing left/right padding applied to fieldset.vertical-tabs-panel.
• Setting word-wrap to normal for div.vertical_tabs.
• Resets thefieldset styles that should not be applied inside vertical tabs.

PS - It's still not perfect because select lists break the layout in IE and still extend beyond the container in all modern browsers, but I'm out of ideas and that isn't directly related to this issue.

Some screenshots are attached.

I like the idea of a variable for this, just so it's not so terribly hard coded.

I'm not comfortable setting it to RTBC. I mean, it looks good to me, but I don't have awesome PHP chops, so it would be great if a developer could review.

Although I'm not sure if this can make it into D7 (because of the late FAPI changes), the Drupal 7 release party in Gothenburg, Sweden will test and review this patch so we hopefully can make some progress on it.

I'm by no way stealing the issue by assigning it to me, just stating that I will coordinate some work on this later today.

No need for me to "hold" on to this issue. Party is over now :)

Bumping this to D8 and subscribing.

#32: drupal-970426-32-fix-fapi-regression-11.patch queued for re-testing.

The patch doesn't look acceptable to me. Highly interested in alternative implementation ideas. Ideally backportable to D7.

I successfully tested width: 100%; max-width: 100%; width: auto; which is IIRC known as an IE-safe legacy workaround.

As a workaround, I apply this in the '#attributes' FAPI property of my custom form elements.

There has been no discussion here since 2015, and work on the patch ended in 2011. The block of code in the Issue Summary was removed from core on Wed May 1 18:43:20 2013 in #1978036: Clean up css in vertical tabs. Based on that I am closing this issue as outdated.