textarea.js assumes #disabled means hiding fields. Conflict with node teasers?

we noticed that different js files are loaded for example on node/add/story than on admin/content/types/add, so maybe this is the source of the problem?

Also in the generated source for disabled textareas there is a <span style="display: none"> wrapper around it, but for those greyed out its not there. To me the second beheaviour would be the reasonable.

This would be the following code in misc/textarea.js doing the hiding:

// Inherit visibility
if ($(this).is('[@disabled]')) {
$(this).parent().hide();
$(this).show();
}

but only on textareas that are resizable.

As a workaround, you could add

    '#resizable' => FALSE,

to your form field, when you set #disabled => TRUE.

Should we always disable resize support for disabled textarea fields?

Should we always disable resize support for disabled textarea fields?

Yes. We should. No need to resize something not usable. With a patch, FAPI should be able to do this automatically if #disabled == TRUE.

I believe this will do.

Well then there is no patch. That's quite different, because CNW will make people presume someone is working on the isssue and that's not so.

Why does a disabled textarea is hidden in the resize JS code at all? Why is the purpose?

I retitled the issue to reflect my understanding of the issue, to be more clear.

Tracked down the origin of the code from the cvs log: http://drupal.org/node/163361

Added a comment there, trying to get people here.

Why does a disabled textarea is hidden in the resize JS code at all? Why is the purpose?

From what I can see, that bit to hide textarea was mainly for node teaser when split. That's all I accounted for in the other issue and that's how it was originally structured. All I changed was what it was checking for. from ":hidden" state to "[@disabled]" property

If it's only for teasers, then this will fix it. Would be better to move it into teaser.js but it's looking a lot more complex in there.

Thanks for taking this on!

- Well, this becomes some lines of code in textarea.js only used on teaser fields, so it should rather go to teaser.js IMHO.
- Generally, the disabled attribute on textares should not be treated as something to hide the textarea, it is supposed to make it read-only but not hide it. So if anything else is dependent on this hiding the textarea, then that should be fixed on its own.

Cross-posted.

Looked further into this and it's not so simple. It's the job of textarea.js to alter the DOM so the resizable grippie is attached to the field. Trying to manipulate it from teaser.js makes it difficult to hide the parent container –grippie wrapper since the teaser function is run first and has no idea about the resizable grippie controller.

Any ideas? I guess that's why it was placed within textarea.js to begin with and maybe it should stay.

I overlooked how the teaser function was called. It can definitely be done from teaser.js. Give me a moment. :)

-----
Edit: I thought it was called with this but apparently it's not so.

from textarea.js:

    // Make sure that teaser.js has done its magic before converting this textarea.
    if (Drupal.behaviors.teaser && textarea.is(('.teaser:not(.teaser-processed)'))) {
      Drupal.behaviors.teaser(this.parentNode);
    }

With that removed, it still works so I'm not sure anymore.

Well, teaser.js uses the same auto-attach mechanism as most .js files in core (Drupal.behaviors.teaser is the function name it is using). Then inside the function, it limits its act to form items matching this selector: textarea.teaser:not(.teaser-processed) (all teaser textareas not processed yet).

Textarea.js uses the same trick, but it selects only textarea.resizable:not(.textarea-processed) HTML elements.

The code you quoted only insures the proper order runtime of the behaviors, always running the teaser.js code before this one, if the teaser JS code is loaded.

If this is to be applied to all teaser textareas, then it looks like it should just be copied to the body of teaser.js, possibly to the end, so the runtime order is still kept. (I have not tried this though).

Thanks, this is new to me.

I did what you suggested. All the teaser specific code moved to teaser.js and did the reverse of what textarea.js was doing to ensure that the resizable grippie is processed right before setting the visibility state. –make sense?

Well, it's working on this end.

Just want to note that the teaser split is broken in Opera 9.25 (mac/win) and it has nothing to do with this patch.

Works fine in IE6/7, Safari 3, FF2.

And here's a fix.

See screen shots before the patch.

Comment didn't sound right. corrected.

dvessel, the spitter bug in opera is also patched here: http://drupal.org/node/201667 fwiw.

Patch looks quite good, but we would need some testing before it gets committed. That dependency ("make sure") code was only required to the hiding, not the wrapping of the elements?

That's true Gábor. I was about to let textarea trigger only when the initial state was to hide but it looked cleaner to just group it on it's own. Functionally it makes no difference I don't think.

And I'm not sure how I missed the other issue. Patch from #18 can be used if it'll be worked on in there.

I did some general testing, and all appeared to work correctly. I'm not familiar enough with the edge cases to know exactly what to look for. If anyone can point me in the right direction, I'll be happy to test them, as well.

Chris, this bug originally showed up when a form was in a disabled state. One way to test is to temporarily hack a form that's attached to the resizable grippie by enabling #disabled then make sure it doesn't disappear when you visit it. It should simply be grayed out.

Here's a random one.. Go into system.admin.module and add the last line:

  $form['site_offline_message'] = array(
    '#type' => 'textarea',
    '#title' => t('Site off-line message'),
    '#default_value' => variable_get('site_offline_message', t('@site is currently under maintenance. We should be back shortly. Thank you for your patience.', array('@site' => variable_get('site_name', 'Drupal')))),
    '#description' => t('Message to show visitors when the site is in off-line mode.'),
    '#disabled' => TRUE, // <- test with this.
  );

That's within the function system_site_maintenance_settings.

Visit http://example.com/admin/settings/site-maintenance and make sure the form is still there.

The other side to this is the node edit form which I'm sure you tested. If they both work then we should be okay. Test on as many browsers as you can. Thanks.

This code looks like it assumes that the teaser textarea is always resizable. So in place of $(teaser).show(); and $(teaser).hide(); you use $(teaser).parent().hide(); only when needed. But that parent only exists, if you have a resizability wrapper, right? That's what textarea.js did: it did show the inner content but hidden the parent.

Hiding the inner content was used so it could be detected later inside textarea.js. Since it's happening inside teaser.js, it doesn't need to do that trick.

And yes, it assumes the teaser is always resizable since it is. Or I missed something.. It's better to be safe so I'll try changing that.

Well, the resizable flag is alterable on the teaser as well, so people might do that on some sites. I guess we need to check whether teaser.is('.textarea-processed'), in which case it is a resizable textarea, and we need to hide the parent, otherwise we need to hide the textarea itself. This looks like the only missing thing here.

join_teaser() & split_teaser() also assumes it'll be wrapped in a resizable text area. Should that be changed too?

I'm guessing it should be left as is after seeing that.

You are right. We are not here to save the world, but to save this #disabled hiding bug, and your patch does a good job in that, so I committed it. If someone thinks teasers should work with or without resizability identically, it entails a bigger effort. Thanks!

On 6.0 RC2 I can no longer see any textareas with teasers because I've set content length to "Unlimited" at /admin/content/node-settings.

It seems that the teaser AND the textarea for main content entry are both wrapped by the span with "display: none" if the teaser is set to "disabled"...

Hmm, re-enabling teasers via setting the content length to 600 characters doesn't fix the problem; I can still see disabled="disabled" on the div with id="edit-teaser-js".

This is strange. The first time I set the length to Unlimited I was able to recreate the problem. But then I set it to a lower limit and back to unlimited the problem disappeared.

Tested on a fresh install and couldn't reproduce. Must have been a fluke.

Just in case someone else hits this: I fixed it on my installation by disabling some modules that I wasn't actively using (devel, coder, and localization client). There seems to be a JS conflict somewhere in one of these modules.