Wide content in fieldsets is cut off

Tested in Opera, looks interesting. I created a superb long menu title and this is what I got on the node add screen. Note the lack of down arrow and actually the select field is not closed, it's clipped indeed.

This is by design. Wide content should be avoided in tableless designs.

This is because of a change in collapse.js. The 5.0 version of the collapse.js is adding 'overflow:hidden' to collapsible fieldsets. The 4.7 version did not. This is a huge problem in the Views administration screen (see http://drupal.org/node/92933).

If you want to preserve the screen layout, you can use overflow:auto instead of overflow:hidden, which should add scroll bars so that the hidden content is still accessible. I'm marking this critical since overflow:hidden makes some pages and forms completely unusable.

I don't notice much jumping, however, as noted above, the clipping does not happen in IE6 even without the patch since IE6 is ignoring the overflow command. So it is a problem only in css-compliant browsers, which means if the jumping is annoying you could make it something that would be ignored by IE6. I know how to do that in regular css but have no idea how to do it in javascript. Then it needs to be tested in IE7 to be sure it works correctly there, since IE7 is more css-compliant than IE6.

However, the jumping does not bother me at all and I would be happy to just leave it set to overflow:auto, since that is the right setting from an accessibility standpoint. You should really only use overflow:hidden on non-critical things like trimming of images that are too large or things like that, otherwise it should be overflow:auto to keep your content accessible.

My reference to the problem in Views in my earlier post was a wrong link. The Views problem is outlined at http://drupal.org/node/97778.

Overly-wide content should be avoided, and any such case in core should be considered a bug. The example in #2 from chx should be truncated if too long.

However, I think this should probably be dealt with so forms that are still wide for some reason are at least visible. I'd like to see another review from Steven and some resolution/confirmation on how good or bad the jumping is.

'Overly wide content' is partly a factor of how the user has their browser set up. If they have a large font size or a small window, things that might not otherwise be 'overly long' will still get truncated. You can't and shouldn't make assumptions about the ways that users might prefer to display their content. Truncating content is horrible for accessibility and I can't think of any reason it should ever be done. The reason for the overflow:auto behavior of adding scrollbars is to make sure content is still accessible instead of cut off.

If you keep overflow:hidden in here, Drupal sites may fail to meet standards for accessible design, which could be a problem for sites that must meet those standards. Because this is a core behavior and not in the theme, there would be no way to create a theme that would not have this behavior.

Looks like this doesn't help IE6.

Sorry for the misunderstanding. I do want to have someone with more expertise than myself review this. I asked Steven.

The patch work as expected, I add only a browser to the webernet list:

Without Patch (display: hidden;):

• Firefox: Wide content in fieldsets is hidden, which can break the UI of some modules such as Views.
• Opera: See Firefox.
• IE6: Ignores the hidden, so it's broken.
• IE7: See Firefox.

With Patch (display: auto;):

• Firefox: Fieldsets with wide content display a horizontal scrollbar inside the fieldset.
• Opera: See Firefox.
• IE6: Ignores the auto, so it's still broken (no change).
• IE7: See Firefox. (Unfortunately a vertical scrollbar also appears.)

Note: Fieldsets without wide content remain unchanged by this patch.

Without patch:

• Konqueror 3.5: see Firefox.

With patch:

• Konqueror 3.5: same as IE7: if the content is too large, you have the horyzontal scroll bar, but unfortunately if the block contains a collapsed sub-block there appears a vertical scrollbar.

Someone with Safari should check this issue too, albeit Safari should use a similar core to Konqueror.

I would like to see it verified if, with jQuery 1.0.3, it is still necessary to apply the overflow style at all. Because of this, we double wrap the fieldset contents in collapse.js.

If we're not going to care about the jumping around, we might as well simplify the code.

Note that there was also a problem before where fieldsets would slowly shrink as you kept opening/closing them repeatedly.

Coming into this late (but reading over the comments so far), I agree with the overflow: auto; approach.

I wonder though, why you don't just use jQuery, detect the width of fieldsets and then apply that width minus, say, 10 pixels to all inputs that have a width great than the width of the fieldset...?

I mean, all this talk about overflow and IE and yadayada can be avoided if you use a universal solution for all browsers that simply detects the width of the container and makes all the inputs inside it less than that width... no?

I wanted to explain how to replicate this issue for new people coming to this.

Start with Drupal 5.0 and add the views module for 5.0. Enable both the views and views UI modules.

Add the tracker view and then edit it. Scroll down the page to the table with many columns and look at the right of the page. I have set up a 5.0 site for this site. I haven't applied the previous patch but can when you are need it.

Kieran

There is a simpler way to reproduce this though not as glaring as: create a new menu called 'something really really really long which will not be able to fit my screens width no matter what' (administer, site building, menus). Now, try to add content (create content, page) and see how the select box lost its down arrow because its cut (if you run a big widescreen monitor you might want to use a non-fullscreen window :) ). This is what I demonstrated in my screenshot in issue number #2 here.

Also, if you want to help but you are intimidated by diff, patch and all that just contact me with your instant messenger (AIM, MSN, Jabber, Skype etc) details and I will be with you as soon as possible :) Or if you are familiar with IRC, hop into #drupal on irc.freenode.net .

Adding this to style.css in garland (approx line 770) fixes it for firefox and konq. And it remains working as expected in IE.

html.js .collapsible select {
  width:100%;
}

It migth be a starter.

and a patch that contains the thing above.

Let's see what Steven has to say about this patch.

No need to wait for Steven, now I took the effort to read the patch -- it only solves the problem for selects and not for tables. But, maybe, something along this line could be a solution... but it needs work.

This behaviour is a function of a content that it too wide for a container. There are two main ways a browser responds:

• respect the width of the container and have the element stick out (correct eg firefox)
• expand the width of the container beyond an explicitly stated width so that it wraps around the wide content (incorrect, ie)

Both of these issues are caused by the content being too wide for the container. This is not the fault of the content - it is as wide as it needs to be - but more a result of the finite width of the browser window.

How do we address it? Well i dont think that we should use the behaviour layer (js) to define the presentation of the document. To have the javascript add style information is a profound decision - this js will affect every theme built on top of Drupal. We really should be using the js to attach classes to elements and have a predefined style in drupal.css. This style would be applied by toggling the class selector in the js. Adding the css style property via the js is just wrong - it makes theme level decisions for all other themes.

The behaviour is never going to be perfect - the content is always going to be too wide for some screens. By allowing theme developers the control over the way their specific theme responds to the issue, we allow much more flexibility in the future, and keep some separation of presentation and behaviour.

FWIW my preference would be for content that spills out of the container, simply because it is the correct browser behaviour, and I feel the usability issues of extra scrollbars outweigh the ugliness of a slightly broken layout.

I vote against CSS modifications with JS.

JS does not respect the cascading in CSS. So, if hve a FOO set to an explicit width, because that works/looks best in my theme, the JS will disrespect my changes and set it to what Drupal thinks it should be. Changing *styles* with JS is about the worst you can do.

However, a better solution, yet still far from perfect, would be to introduce a new class "fullwidth". JS can then dynamically apply (as in: add to the existing classes) that class to any element inside, for example, collapsible fieldsets. It is then up to the one writing the CSS to use that. The default in Drupal.css could be .fullwidth {width:100%;}

The trick as outlined above in my patch (it was only meant to show the trick, not to be a patch for review) is to explicitly set the width to 100%. This needs to very particular, else we accidently set stuff on elements: One of the biggest frustrations for theme developers with Drupal is that some style used for something else suddenly breaks their layout.

So, pragmatic: Any element that grows too large inside a problematic item needs to get a width: 100%. Possibley by assigning a new fullwidth class, possibly by simply hunting down all broken elements one by one and define a rule for each of them.

Re #19 and #20, couldn't we remove this completely from the js and then make necessary adjustments to the css for each of the core themes to handle overflows appropriately? The 4.7 version of the js didn't do this, so this was a deliberate change and there may be a reason why someone felt that this needed to be done in js instead of css. If so, it would be nice to hear the rationale.

To move the handling of overflow completely out of the js and into the theme, the following seems to work:

1) Completely remove the line in the js that attaches the overflow behavior.

2) Add the following css to all core themes, or any that you want to behave this way:

/* Avoid jumping around due to margins collapsing into collapsible fieldset border */
html.js fieldset.collapsible div {
  overflow:auto;
}
/* Make sure overflow behavior attaches only to top-level wrapper div in fieldset */
html.js fieldset.collapsible div div {
  overflow:visible;
}
/* Keep scroll bars from displaying when not needed by shrinking tables and forms a bit */
html.js fieldset.collapsible div table, html.js fieldset.collapsible div form {
  width:98%;
}

The first rule applies overflow:auto to the wrapper div. The second rule keeps that from applying to nested divs below the wrapper div. The third fixes a small problem in some browsers like firefox when 100% width tables are inserted into collapsible fieldsets since they are just slightly too wide so they get an unnecessary horizontal scroll bar.

Change the first rule to overflow:hidden and all themes will look exactly the same as when this was done in the js.

By doing it this way instead of in the js, it is possible to create a new theme that keeps the overflow visible, if that is desired. Or people who know a bit of css can easily figure out what to change in the core themes if they want to change the display.

I can't figure out any way in Tortoise to create a patch for a js file and a number of css files that are all in different folders but which have the same names, so I haven't created a patch, but it's pretty easy to see what to fix.

I've tried this in Firefox and IE 6 and 7 and it seems to work. It's possible a bit of tweaking will still be needed in some themes or for other browsers, but it shouldn't be really broken anywhere, as it is now.

#34 fixes the problem for me in FF 1.5 and FF 2. Now I can stop having to turn JS off in order to use Views. I think the lesson learned is that we need to keep as many CSS properties and modifications in CSS and avoid turning to JS for layout tweaks.

I leave this up to Steven to decide/commit.

Seems to me like the most common and best way to get around things.
Though for D6, I think we should really have a max width for table content and a maximum number of rows..

The patch in #34 breaks in Safari 2. Attached is a screen shot. For nested collapsible fieldsets, a vertical scrollbar appears for no obvious reason.

Okay, The double wrapped DIV's were removed. I've only checked this in Safari v2 & Firefox 1.5. The only minor thing I ran into are random horizontal lines in the fieldset.

I'm sure it needs more work. Pleaser review.

Oh, and the scroll bar in safari is addressed in the patch with the ugly margin & padding canceling each other out. Might be a better way but I was in a hurry.

The ugly padding/margin cancellation causes lines to appear behind items in IE6.

This patch removes the negative margin and just uses padding. Investigation shows 4px is the minimum amount of padding necessary to remove the scrollbars, so that's what is set. This could probably be wrapped in some Safari specific CSS, but I haven seen the Safari equivalent of the Tan hack, so the padding is set for all browsers.

Otherwise, this patch is basically the same as dvessel's.

This patch comes courtesy the TC Bug Hunt

Rewrote the whole thing. Tested with IE 7, Safari, Firefox 2.0.

Just some notes on kkaefer's patch: I think there's already enough space from the upper edge to the contents as it is. Adding the 4px padding inside: "html.js fieldset.collapsible div.fieldset-wrapper" causes even more. It can also break if a theme globally scales with em's or other relative measures.

The scroll bars in Safari comes from the "margin-bottom: 0;" set inside "html.js fieldset.collapsed". How about only settting a *bottom* padding of 1em for "html.js fieldset.collapsible div.fieldset-wrapper" since it's always the collapsible fieldsets at the bottom causing the Safari scroll bars. I don't remember running into a collapsible fieldset as the first child of another fieldset and 1em will play it safe. I think the extra bottom padding is ignorable.

html.js fieldset.collapsible div.fieldset-wrapper {
  overflow: auto;
  padding: 0 0 1em;
}

Can't judge if the rewrite is better but it does cause horizontal scroll bars to show right before the page finishes loading.

Just kkaefer's patch with the style I noted. Made the bottom padding .5em instead of 1em.

And disregard my previous note on the first child of the fieldset being a collapsed fieldset. Even if that occurred, it wouldn't affect Safari since it was the bottom padding of the collapsed fieldset causing the scroll bar. Excuse my wordiness. :p

And the window's scroll bar appearing right before it finishes loading happens in Safari. Minor issue, but if it can be prevented it should be.

And disregard my previous note on the first child of the fieldset being a collapsed fieldset. Even if that occurred, it wouldn't affect Safari since it was the bottom padding of the collapsed fieldset causing the scroll bar. Excuse my wordiness. :p

Eh, I meant the "bottom-margin: 0;" in the collapsed fieldset causing the scroll bar. -wished comments were editable.

Just realized it was a theme specific issue. The properties set by Garland was causing the problems. No padding required. :\

Aye, at least we know what Garland has to change with this patch. Padding removed with this patch. Sorry for the confusion.

Awesome work guys. Tested in Safari, Firefox, Opera. There is no unnecessary jumping or such.

Committed to HEAD.

As noted in #38, this patch adds an unnecessary scrollbar for nestedfieldsets on Safari. Attached is a screen shot from the views page.

To reproduce this problem:
- update to latest HEAD, where patch was applied
- install HEAD of views.module
- visit admin/build/views/add/frontpage

Comments elsewhere indicate the problem is with Garland. I'm going to investigate this and provide an updated patch if possible.

oops. screenshot attached this time.

As noted in #47, this problem only occurs in Garland.

The attached super simple patch adds some additional padding to the bottom of fieldsets in Garalnd (and therefore Minnelli) to eliminate the scrollbar.

No rollback is necessary. Just apply the patch to HEAD.

My previous patch used px. The scrollbars would reappear on font size changes. Rerolled using EMs. Experimentation shows .6em was the smallest value I could use to solve the problem.

Rerolled with -u. Time to alias that command....

Committed to CVS HEAD. Thanks.

This change is definitely an improvement. However, when loading content through AJAX, we sometimes need to reattach behaviours. The practice of naming an attach function facilitates this, so putting the attach in an anonymous function was a step backwards.