Support from Acquia helps fund testing for Drupal Acquia logo

Comments

HasseMan’s picture

Looking at wysiwyg.module this is what I find:

wysiwyg_process_form(&$form)
  wysiwyg_add_editor_settings($profile, $theme)
    $config = wysiwyg_get_editor_config($profile, $theme)
      $settings = $editor['settings callback']($editor, $profile->settings, $theme)

Hope that helps a bit.

sun’s picture

Status: Active » Postponed (maintainer needs more info)

Why do you want to configure the width and height per field in the first place?

And why can't you use CSS to adjust the width of the surrounding container?

rconstantine’s picture

Why do I want to do it? One word - "multigroups". If you haven't tried them yet in CCK (currently in version 3.x), then you're missing out! When I get in to work tomorrow, I'll show a screenshot of what I'm working with. But for now, I was able to find a work around of sorts.

First, there was a bit of code which specified the height of all WYSIWYG boxes to be 400px, which I commented out. This has the result of using the fckeditor default height of 200px, which was better for me.

Second, since there is code which I mentioned before that specifically disallows the changing of the height/width of a WYSIWYG box, I instead made the text areas the last item in the multigroup row and then specified the widths of all of the items before it. This allowed the text area to fill the remaining space. I would imagine though, that there could be times where it is not convenient to reorder form elements to make this accommodation.

If you don't know what code I'm referring to when I say that there is a function that disallows the changing of the width and height, I'll look it up for you. There is actually a comment there in the code that says 'prevent', or 'don't allow', or 'disallow' or something like that. You could do a search. I don't recall whether that was in fckeditor itself or WYSIWYG. I think it was in fckeditor since the dimensions are supposed to be passed in as constructor parameters. So... what I really need is a way to set the constructor parameters for each instance of the WYSIWYG.

Make sense?

sun’s picture

well, multigroup or not - do you think that admins will (and want to) configure editors separately for every single field in their Drupal site?

rconstantine’s picture

obviously, you give them the default that now exists. But really, yes, I do think admins want this, which is why most of the basic CCK fields have width/length settings. I mean, the standard text area allows you to set the height, which this module also doesn't honor.

I realize that others haven't chimed in on this, but then again, this post isn't very visible. Without multigroups, which is in the 3.x version of CCK now and will be official soon, one can always move the text area to another line. With multigroups, you can't.

Try multigroups before you decide one way or the other.

escoles’s picture

Configuring the height of fields is very important. If you need to create a CCK field which needs to contain a small amount of formatting information, you'll want to be able to keep those fields from blowing up your edit form vertically.

On every site we do, I configure at least five additional filtered-text fields for every node that is ever rendered as a page:

  1. "Kicker" or "eyebrow" headline, to be displayed above the title, which must be able to include markup and special characters.
  2. Alternate "formatted title", for cases where the Title needs to include markup or characters that can't be included in the standard node TITTLE field.
  3. Secondary headline, displayed below the Title, which must be able to include markup and special characters.
  4. Side Content field, to be displayed in the sidebar.
  5. Abstract (Teaser) field, to serve as a teaser.

The first three need to be shorter than body copy fields, or you have the following problems with users:

  • Users are already intimidated by the many-page vertical scroll of a node edit form that includes extra fields and features; if you can't shorten the edit box, it gets even worse.
  • There's also a usability issue in that with fields having related function (e.g., the Formatted Title over-rides the Title), the very large boxes tend to force the fields onto different screens from one another.
  • They are liable to see the large size of textareas as a tacit encouragement to add too much text or do formatting that's not appropriate to the layout.

Short version: This is just something people need to do. Other editor modules support it. It's a significant usability problem to not have it. WYSIWYG is not a viable option for any site my company implements until this feature is included.

hadsie’s picture

Just to add to the discussion... being able to set the dimensions of each field is quite an important feature. I often have textarea's that should only be 2 lines long, and others that should be significantly more, depending on the context.

"And why can't you use CSS to adjust the width of the surrounding container?"

@sun - I'm not sure how to make this work. I'm using fckeditor and have been playing around with the CSS for quite awhile to get it to the right height. In FF this isn't a problem, but in Safari the scroll bar goes far beyond the end of the textarea when I do this. Basically the surrounding container's height is shorter, but the inner container isn't respecting that in Safari at least. How can the height be set properly using CSS? I can attach a screenshot if that would help.

- Scott

hadsie’s picture

Also, I notice that this has been set to "postponed (maintainer needs more info)" for quite a while. What info do you need for this to move forward?

hadsie’s picture

FileSize
1.42 KB

Here's a patch that's basically just a quick hack using the $rows attribute of the textarea field. I'm not sure how reliable converting rows into pixels is, but it seemed to work ok for me.

sun’s picture

Title: Set width for each field » Allow individual width/height per field
Component: Editor - FCKeditor » Code
Status: Postponed (maintainer needs more info) » Active
drupalninja99’s picture

Any headway on this? It seems like the FCK editor should adjust height based on the number of rows in the textarea so we don't have to do this granular adjustment.

sun’s picture

I think I mentioned in another issue that a bug related to FCKeditor not always respecting the textarea's height got fixed recently in the FCKeditor library. Could you try with the latest version and report back?

drupalninja99’s picture

im using the latest version of fck editor - http://ckeditor.com/download, ckeditor is not supported right? or i should say with imce. i want whatever editor is going to work with imce or some kind of image uploader

sun’s picture

If you ask that, then you are at least not using the latest version of Wysiwyg module. ;)

drupalninja99’s picture

I'm using version 6.x-2.1, what am I supposed to be using?

adeb’s picture

Subscribing.

I can change the width by overriding CSS values for CCK text area's which get a unique id e.g. #edit-field-textarea-0-value-wrapper.
But this will give problems with multiple content types that need different widths.

edit: created a unique body class for each content type which allows me to target each individual content type.

HnLn’s picture

subscribe (using ckeditor)

willieseabrook’s picture

I have a similar issue, which perhaps could fall into a different request, but I'll put it here and leave it for you to decide.

I'm using WYSWYG with CKEditor

A project I'm working on requires very customized node edit displays, that differ substantially per cck node type. Substantially not just in CSS look and feel, but in layout and additional form elements.

I want to be able to customize each CKEditor depending on what the node type is, and again depending on what display that is.

I'm using

module_wysiwyg_editor_settings_alter(&$settings, &$context)

to alter the width and height settings using arg(n) checks to see where I am.

But one would think you could add a 'node' index to the $context array - so that a coder can more cleanly make adjustments to the size and shape of the editor depending on node values.

willieseabrook’s picture

Thought about this more.

Would actually be smarter to append the $form object somehow to the $context, as that would deal with more situations.

HnLn’s picture

I agreee with #11, the editors heigth (and possibly width) should adjust to the nr of rows (cols) provided in cck.

kndr’s picture

Status: Active » Reviewed & tested by the community
FileSize
1.9 KB

#9 works for me with TinyMCE editor. Thank you! I am attaching modified #9 patch for TinyMCE and FCKEeditor.

ksenzee’s picture

This approach makes sense to me - rather than deciding all textareas must and shall be 420 pixels high, why not estimate based on #rows?

I'm attaching a reroll that adds ckeditor support and fixes minor code style issues.

TwoD’s picture

+++ wysiwyg.module	23 Jun 2010 00:44:35 -0000
@@ -170,6 +170,10 @@ function wysiwyg_process_form(&$form) {
+              $profile->settings['height'] = $field['#rows'] ? 40 + 29 + 15 * ($field['#rows'] - 1) : 400;

Maybe we should do this calculation in the editor implementations and just store the number of rows here?
The reason being we don't know how many toolbar rows there will be. (Actually, the implementations don't know that either yet, so I think we could change that later if needed.)

+++ wysiwyg.module	23 Jun 2010 00:44:35 -0000
@@ -170,6 +170,10 @@ function wysiwyg_process_form(&$form) {
+              ¶

Tab. ;)
No need to reroll for that.

Powered by Dreditor.

sun’s picture

Status: Reviewed & tested by the community » Needs work
+++ wysiwyg.module	23 Jun 2010 00:44:35 -0000
@@ -170,6 +170,10 @@ function wysiwyg_process_form(&$form) {
+              // Rough estimate of textarea height in pixels:
+              // 40 for the toolbar, 29 for the first row, and 15 for the rest.
+              $profile->settings['height'] = $field['#rows'] ? 40 + 29 + 15 * ($field['#rows'] - 1) : 400;
+              ¶

1) We should make sure that modules that are altering profile settings can override this value. Not sure when the drupal_alter() actually happens, but ideally of course, the pre-calculated height should be contained already (i.e., making the alter come afterwards).

2) Looks like $field['#rows'] is presumed and required to exist? Possibly the test should have been !empty().

3) Do I need to understand the -1 (minus one)? A field having 1 row will lead to a height of 0 (zero).

4) MONSTERTABMONSTER.

Powered by Dreditor.

TwoD’s picture

Status: Needs review » Needs work

1) I assumed the alter hook would be called after this, I might be wrong though, need to check the code.

2) You're right.

3) "29 for the first row", 1 row means 40+29+(15*0) pixels. This is part why I wanted to move this calculation to the implementations, those constants may not be valid for all editors.

4) =P

EDIT: Had missed a quite important little word, see highlight above.

avner’s picture

subscribing

Anonymous’s picture

The methodology in #22 is mostly working for me with ckeditor. My only problem right now is that a CCK multi-line text field is not honoring the number of rows in a node creation form. I'm using the Rubik theme if that matters (not sure why it would). The strange thing is that TinyMCE seems to honor the number of rows properly within the same form.\

Any thoughts?

mcarbone’s picture

This also doesn't seem to be working if there are multiple fields on a single form w/ different row values. For instance, I have 4 textareas on a single form, but wysiwyg_ckeditor_settings is only being called for the first one (and the rest of the textareas are using the same settings).

mcarbone’s picture

Oh, this is happening because of this function:

function wysiwyg_add_editor_settings($profile, $theme) {
  static $formats = array();

  if (!isset($formats[$profile->format])) {
    $config = wysiwyg_get_editor_config($profile, $theme);
    // drupal_to_js() does not properly convert numeric array keys, so we need
    // to use a string instead of the format id.
    drupal_add_js(array('wysiwyg' => array('configs' => array($profile->editor => array('format' . $profile->format => $config)))), 'setting');
    $formats[$profile->format] = TRUE;
  }
}

It's caching by format, not by a particular field's format. Since these could each be unique, and fields shouldn't repeat, this needs to be rewritten so that it works on a per-field basis and not a per-format basis.

mcarbone’s picture

Status: Needs work » Needs review
FileSize
7.23 KB

OK, here's a patch that addresses the issue I mentioned in #29.

Re: sun's first issue in #24: the drupal_alter comes after the height computation here, so it's not an issue.

emilyf’s picture

@mcarbone, thanks!! initial testing with your #30 patch seems to be working great.

szantog’s picture

This patch in #30 worked with ckeditor.
Thank you!

3dloco’s picture

#30 works with ckeditor for me too. Thanks a lot!!!!

my-family’s picture

Status: Needs work » Needs review

#30 works nice (tested for FCKeditor 2.6.6. so far), resolved my strong headache, thank you!!!

ayalon’s picture

Status: Needs review » Reviewed & tested by the community

#30 <-- works perfekt!

This is the fourth test and considered to be rtbc.

I tested this with ckeditor and fckeditor.

Very good patch!

gooddesignusa’s picture

patch #30 worked for me. I'm using recommended version 6.x-2.1. Had some offset's show up when running the patch but it still worked. Thanks a lot

sun’s picture

Status: Reviewed & tested by the community » Needs review

Thanks for working on this! However, it seems like this patch is now touching logic that is essential for all editors. It doesn't look like it has been tested with any other editor than (F)CKEditor. Those changes require some careful review, but also testing all other supported editors.

gooddesignusa’s picture

It seems to work for me using CKEditor. I am only using it for 1 field and I dont allow them to add another. It is also very limited with buttons like bold, italic, underline, etc. I haven't seen any issues yet but haven't tried to do anything out of the normal for a basic user.

szantog’s picture

Status: Needs review » Needs work

Hmm, this is not good..
If we need this feature, it sould eliminate the wysiwyg profile cache system complete, if the a field has #row attribute.
This happens, if preview something: #939258: ckeditor disappears if pressing preview

I don't know, what would be the best, maybe a checkbox in profile edit form, to enable resize editor made on textarea #row settings..

tallsimon’s picture

For CCK fields there is this patch for the standalone CKEditor - it adjusts the height of editor by CCK field setting http://drupal.org/node/869116

3dloco’s picture

Seeing same issue mention in #39 (CKEditor disappear when user is previewing a node as a consequence of patch at #30)....:-(

mrwhizkid’s picture

I can confirm this problem. Any suggestions for how to get around this? The width patch is very important but so is this issue of the node preview. Thanks for your help.

Danny Englander’s picture

#30 worked like a charm but crucial to note that it only worked against 6.x-2.1. This solved a lot of headaches for me. thanks.

Dave Reid’s picture

Subscribe this is really useful. I'll work on re-rolling this for 7.x-2.x tonight.

TwoD’s picture

@Dave, would you mind moving the hard-coded constants added to the calculations out to the editor implementation files while you're at it? That way the height/width can accomodate for variations in toolbar heights etc between editors.

I haven't tried the patch in #30 yet, but one of the things worrying me is that on a page with a lot of format-fields, Drupal.settings.wysiwyg.configs might become huge as all format settings are duplicated per field, except for the width/height. (Sorry if I'm misinterpreting the patch.)
I would rather have per-field settings stored somewhere separate (maybe Drupal.settings.wysiwyg.configs.fields[field][editor], if they aren't going to be different per format). Or something like the global per-editor settings, so these can be merged together from "normalized data pools" (thinking of normalized SQL tables here) with as little duplication as possible.

Sidenote:
That reminds me of a possible issue in D7 now that formats have machine-names... currently they are all stored with a 'format'-prefix, but if we eliminate that we'll get issues if someone names their format "global". Maybe we should keep prefixes after all...

sun’s picture

Version: 6.x-2.x-dev » 7.x-2.x-dev

I share the concern about duplicating the entire editor settings.

Long-term, we need an inheritance cascade of global » format » field settings. For this issue, we need to introduce format » field overrides.

Danny Englander’s picture

Is there a way to tell if the patch has been rolled into the latest 6.x dev version?

TwoD’s picture

@highrockmedia, the easiest way to determine that is by looking at the status and version fields. We roll new features/fixes for 7.x-2.x-dev when an issue reaches RTBC, then we backport to 6.x-2.x-dev (if this doesn't happen immediately the issue is marked "patch (to be ported)") and finally mark it "fixed" once committed to all relevant branches.
So, this patch has not made it to CVS for any version yet, because of the problems mentioned in #45 and #46.

Danny Englander’s picture

@TwoD - thanks for the info. :)

Encarte’s picture

subscribe

ayalon’s picture

I tried to fix the issues, but failed. We need some help from an experienced maintainer. #46 is not enough clear to me.

NPC’s picture

Those recommending to use CSS for width, can you please elaborate which id / class it could be applied to for CKEditor? I successfully adjusted the height by the following (making editor less bulky for the comment entry):

.comment-form .cke_contents {
    height: 150px !important;
}

But this does not work for width, and there are so many layers of encapsulation, divs upon divs, and I may be missing something - applying width to some elements does not help, applying it to others breaks the editor.

Thanks for any help!

scott.whittaker’s picture

Subscribe

yugongtian’s picture

+++

lightstring’s picture

Subscribing

ailgm’s picture

I've manually applied the patch in #30 to Wysiwyg 6.x-2.4 with CKEditor and encounter this error on any page with a Wysiwyg field:

Fatal error: Cannot use object of type stdClass as array in .../sites/all/modules/wysiwyg/wysiwyg.module on line 367

That's the line:
drupal_add_js(array('wysiwyg' => array('configs' => array($profile->editor => array('format' . $profile->format => array($field['#id'] => $config))))), 'setting');

The patch appeared to be designed with Drupal 6.x. Have I overlooked something, or does this not work with the 2.4 version?

Mark Vincent Verallo’s picture

My quick and dirty solution:

  $form['MYTEXTAREA'] = array(
    '#type' => 'text_format',
    '#attached' => array(
      'js' => array(
        'MYTEXTAREA = setInterval(function() {           
           if (jQuery("#cke_contents_edit-MYTEXTAREA-value").length == 1) {
             jq("#cke_contents_edit-MYTEXTAREA-value").css("height", "100px");
	     clearInterval(waiter);
	   }
        },1);' => array('type' => 'inline', 'scope' => 'footer', 'weight' => 5)
      )
    )
  ); 

Change '100px' to your desired height.

AndyF’s picture

subscribe

JustMagicMaria’s picture

Subscribe. (Using ckeditor and 7.x and could really use this feature.)

tchopshop’s picture

Subscribe... for D7 and CKeditor

bforchhammer’s picture

Subscribe... for D7 and CKeditor

Ditto.

Danny Englander’s picture

@DrupalRevisited -- Just curious about your solution in #57. Where and how do you implement this? Is it a module, in template.php or are you hacking an existing piece of code that could get overwritten later?

chichilatte’s picture

Yep, patch #30 alone works for me! Using D6.22, wysiwyg-6.x-2.4, CKEditor library 3.6.2
Serious thanks mcarbone. Ups the usability 10fold!

esbite’s picture

Patch #30 works, though as stated above it breaks the node preview, but that's not an issue for me. Many thanks!

Attaching the same patch, but made with git and slightly modified to work with wysiwyg-6.x-2.4.

ksenzee’s picture

I understand and agree with the maintainers' concerns in #45 and #46 but I don't know the module well enough to do that kind of refactoring. So in the meantime here's a version of #30 rerolled for Drupal 7, for those who need it. Conveniently it doesn't break node previews anymore. I'm guessing that's because #208611: Add drupal_array_merge_deep() and drupal_array_merge_deep_array() to stop drupal_add_js() from adding settings twice got fixed in core. So the only problem I see with the D7 version of the patch is the architectural concern.

drupal a11y’s picture

Check the following posts for a solution:
http://drupal.org/node/1332210 -> D7 version for http://drupal.org/project/textarea_expander

And this issue from the D8 core: http://drupal.org/node/432730

Anonymous’s picture

#65 worked for me on 7.x-2.1. Note that you need to use the -p1 switch on the patch command i.e.

cd sites/all/modules/wysiwyg
patch -p1 < path/to/patch/file

Thanks guys.

mpotter’s picture

Status: Needs work » Reviewed & tested by the community

This patch (#65) worked for me and was VERY useful on my site!

(git apply works fine without any -p1 btw)

gaele’s picture

Status: Reviewed & tested by the community » Needs work

The patch from #65 does not work for textfields. Textfields have no row setting, though they will always be one row high.

gaele’s picture

Status: Needs work » Needs review
FileSize
6.19 KB

Patch from #65 adjusted to work with textfields (which will have a height of one row). Also moved hard-coded constants out to the editor implementation files, as per #45, first remark.

Jason Dean’s picture

#65 worked for me on standard node fields, but caused problems when using Field Collection module.

Say I have a field collection defined with a long text field and using WYSIWYG formatter. I can create a node and fill out the first field collection entity. When I click to 'add another item', the WYSIWYG editor disappears and the field blanks out.

Reverting to the unpatched WYSIWYG module resolved the problem.

David_Rothstein’s picture

I found myself in a situation where I needed to use the patch in this issue alongside the one in #356480: Lazy-load editors.

It turns out they don't play nice together, but if you apply the additional patch I've attached here, that makes it so they do. (Since that other issue looks like it will probably be committed soon, the attached patch will need to be merged into this issue's main patch once that happens. But for now, it's only needed if you're trying to make both patches work together correctly.)

anou’s picture

Hello,
I must agree with #71 and I will add that when you save your node using Field Collection fields, the textarea won't get saved.

I'm using #70 patch.

Reverting solved the problem. Must live for the moment with my text area's height not configurable.

mstef’s picture

#65 did a good job with the height..

dkingofpa’s picture

Patches in #65 and #70 failed to apply with git. Had to use patch -p1 < wysiwyg_per_field.patch to pull in the code from #70. Re-rolled the patch in #70 against the latest 7.x-2.x branch. Patch generated with git diff > wysiwyg_height_per_field-507696-75.patch and confirmed that it applies correctly using git.

This functionality is great...no more gigantic wysiwyg editors that completely ignore the field's rows setting.

sassafrass’s picture

Applied patch in #64 and it worked perfectly, but had to apply it by hand. Using version = "6.x-2.4" with ckeditor. Thanks!

stevenovy’s picture

Applied patch in #75 and field row settings for textarea fields are working great with CKEditor. Thanks all!!

loze’s picture

Confirming patch in #75 works for me. Thank You!

greggmarshall’s picture

While I had to apply the changes in the patch manually, adjusting for some code shifts, the code in patch #75 also appears to work with CKEDITOR on version 6.x-2.4. I did not patch the other editor include files.

samerali’s picture

#64 worked great, thanks!

ayalon’s picture

#75

Confirming that #75 is working as expected and applies without error.

zhangtaihao’s picture

#356480: Lazy-load editors has landed. The now superfluous extra pre-initialization has been removed. @David_Rothstein: the other patches will now work without conflict resolution.

vegardjo’s picture

#75 is working-ish for me, but there are some issues with Field collections as mentioned in #71 and #73. This might be an issue for all fields where number of values are set to "unlimited" (so you get the "add another item" button), but haven't tested that.

* The editor is there when you start, but after you click "add another item" the textarea (with content) for the first item disappears completely
* The new textfield in the new collection is there, but with no editor
* I am not able to add a third collection after this
* ..however, I can save the node and the two first collections are saved. On editing the node again I can add one two more, save, add two more etc..

Console log says:

Uncaught [CKEDITOR.editor] The instance "edit-field-faktaboks-und-0-field-faktatekst-und-0-value" already exists. ckeditor.js:25
Uncaught TypeError: Cannot read property 'document' of null ckeditor.js:20

..if I set an absolute number of Field Collections, say 3, everything works just fine!

edit: screenshot here:
https://www.evernote.com/shard/s138/sh/ee3f6b9e-fe05-4eb2-8dee-da8425270...

prinds’s picture

With the newest version (7.x-2.x-dev) and the patch in #75 the height is set correctly..

But as a nasty sideeffect all buttons are showing, ignoring the choices I made in the editor settings.

Anyone else seeing this?

prinds’s picture

It turns out I just needed to clear caches to get the buttons back as they were before the patching.

So I can confirm that #75 is working when caches has been cleared.

Thanks

gaele’s picture

So, #75 is working, except when using a Field Collection field in combination with the "Add another item" button.

I tested this:

  • the patch from #75
  • wysiwyg 7.x-2.x-dev 2012-Nov-11
  • field_collection 7.x-1.0-beta4
  • CKEditor 3.6.5
  • Firefox 16.0.2 and Chromium 22.0.1229.94 Ubuntu

Results:

  1. This did work using text areas. No problem adding another item.
  2. This did not work using a text field. Adding another item resulted in the first text field being empty (the editor itself on that first text field is shown though).
brettbirschbach’s picture

Subscribing.

Was super excited to find this thread, but once again field_collections strikes again. I swear, field_collections is the most popular yet poorly supported module in Drupal 7.

brettbirschbach’s picture

So I tried my hand at debugging the issue in #86 - Text Fields in Field Collections with WYSIWYG editors.

I wasn't able to fully figure it out, but I think I have some good observations that someone may be able to leverage in narrowing down a solution.

The problem seems to be that in the UI, the data is not transferring between the WYSIWYG editor and the actual text field in the background. So if I enter "Hello World" into the WYSIWYG, and then switch to "plain text" mode which displays the text field (an <input> control) the value is empty. Further, if I enter "My name is Joe" into the text field, and then switch back to the WYSIWYG editor, it will still display "Hello World" instead of my new value. If I save the node, "My name is Joe" will be successfully saved from the text field. On viewing the node again, it will appear that the value is not there if I am looking at the WYSIWYG editor, but if I switch to plain text mode the value will indeed be there.

So as I said before, it appears the issue is with communication of the value between the WYSIWYG editor and the text field. I think a very indicative symptom of the problem is something I noticed in the HTML (using firebug) after modifying the value in the WYSIWYG editor and then switching to plain text mode. Though the "value" of the <input> field is not updated to match the WYSIWYG editor value, the value of the WYSIWYG editor is inserted as a node to the <input> tag. i.e. instead of <input value="val from wysiwyg"/>, I get <input value="old value">val from wysiwyg</input>

So something in ckeditor is making it think that it gets and sets the value of the Text Field by getting/setting the innerHTML of the node instead of the "value" attribute. When I look at the ckeditor.js library (I have 3.6.3) I see some code that leads me to believe this is indeed happening - i.e. l=m.is('textarea')?m.getValue():m.getHtml(); Note that this field is an 'input', not a 'textarea', so this logic will indeed pull the innerHTML instead of the value.

This is as far as I am able to take it at this point, due to time constraints and the fact that I dont have the time to try to figure out how to generate an uncompressed version of ckeditor.js to better debug. I'm hoping these clues can help someone else figure it out.

brettbirschbach’s picture

Adding to my comments in #88, a potential other clue is that if I comment out the following lines of the patch:

elseif ($field['#type'] == 'textfield') {
$profile->settings['rows'] = 1;
}

It appears that my Text Field somehow picks up the row value of the Text Area field below it, within the same Field Collection.

Anonymous’s picture

the patch from #75 is no longer working with the latest dev (2013-Feb-23)

gaele’s picture

Status: Needs review » Needs work
Grayside’s picture

#75 still works for 7.x-2.2

pedrosp’s picture

For all Drupal 6.x'ers still around: I can confirm that you can apply modifications from #75 patch on 6.x-dev "manually".
That is, looking at the patch code, and change wysiwyg.js,wysiwyg.module and ckeditor.inc.

mstrelan’s picture

#75 is great, although not compatible with #741606-45: Teaser splitter / text fields with summary support.

mstrelan’s picture

As a follow up to #94 the attached patch adds rudimentary support for setting the height of summaries. Note that you have to have previously applied the patch in #75 and the patch referenced in #94.

WorldFallz’s picture

#75 no longer applies-- here's an straight reroll against the current dev.

ShaunDychko’s picture

#96 works great. Thank you.

ShaunDychko’s picture

Issue summary: View changes
Status: Needs work » Reviewed & tested by the community
Anonymous’s picture

There seems to be a problem with #96 with current dev. There is no wysiwyg.js file in the js/ directory

Anonymous’s picture

Also, changes listed in #96 to wysiwyg.module cause WSOD.

juliusvaart’s picture

#96 works great. Also on Field Collections.

czigor’s picture

@trainingcity: In my wysiwyg dev there is no js/ directory at all. The wysiwyg.js file is in the module root.

The patch applies cleanly and works great.

anou’s picture

I can confirm that patch #96 works great, even with my module's custom fields! ;-)

Anonymous’s picture

Anyone know if a similar patch is available for the CKEditor Module? I prefer it over WYSIWYG Module as it allows me better control over the menu button layout.

ayalon’s picture

ayalon’s picture

jenlampton’s picture

It looks like the patch in #106 was mistakenly created from the drupal root, not the wysiwyg directory.

I can confirm that the patch in #96 does apply cleanly to the dev version, and continues to work great.

+1 for RTBC on #96

broeker’s picture

Update: patch #96 no longer applies to latest dev, and patch #106 (as mentioned) was created from Drupal root instead of the wysiwyg directory (it also causes a fatal whitespace error from last file line).

bneil’s picture

Status: Reviewed & tested by the community » Needs work

Updating to needs work per #108

Anonymous’s picture

Anonymous’s picture

Status: Needs work » Needs review
Anonymous’s picture

FileSize
6.16 KB

Fixed last patch - missed one line when re-rolling patch.

hwasem’s picture

I tested #112 against latest dev (Wysiwyg 7.x-2.2+33-dev (2014-Mar-19)) and it failed on hunk#2 at 471.

Anonymous’s picture

FileSize
6.21 KB

Hmm, weird. Re-rolled again and tested against 33-dev. Applied clean.

hwasem’s picture

That works for me now. It look care of a directory path (page not found) issue I was getting in my error log for font.css and micro fonts in my wysiwyg field areas.

Thanks so much!

Vacilando’s picture

Status: Needs review » Reviewed & tested by the community

Applies cleanly and works beautifully for CKEditor within WYSIWYG!

A bit on a limb here but given several positive tests and the fact that the last few problems only related to patching I set this to RTBC.

rjacobs’s picture

This is a great patch and a huge editor/admin UX improvement.

From a functional perspective I can also conform that this applies and works well. However, given that it makes adjustments for multiple wysiwyg editor flavors (ckeditor, fckeditor, tinymce) it might be useful to the maintainers if we can confirm that #114 works correctly for each. It does not look like the confirming reviews above note which editors have been checked. Currently I can confirm that things work great for ckeditor, and a look at the patch makes me pretty confident things are fine for the other editors (after all they each appear to just take a px "height" config param), but it might not hurt to confirm that.

From a code perspective the changes seem pretty innocuous, though it's not clear to me why a couple conditional checks were removed from wysiwyg_add_editor_settings(). Comment #29 alludes to the changes in this function, but it does not explain the final changes made. Perhaps there were just a couple unneeded sanity checks in there that were safely pulled out?

rjacobs’s picture

Hummmm, looking more it's not clear if the issue about Field collections was resolved (#71, #73, #86-89, etc.), or even if that would be a show stopper for this.

Also, I assume the concerns from #45 are non-issues now? I think that was back when this was against D6, so maybe it's irrelevant? This issue is so old it's really hard to keep track of things and do a comprehensive review.

I'm not going to change the status, but wanted to raise these questions in hope that they can be addressed before becoming commit-blockers.

TwoD’s picture

Status: Reviewed & tested by the community » Needs work

Hmm, the patch is no longer passing settings through the new processObjectTypes(), so it would no longer be possible to use settings which require Function references or RegExp values. For example, TinyMCE 4 changed their file browser callback setting from being a string in TinyMCE 3 to an actual Function reference, so we need this for the TinyMCE 4 support patch to not break compatibility with IMCE Wysiwyg bridge, and thus IMCE.

Also, I'm starting to think that it would be much handier to have this implemented in the .js files, in the attach methods specifically. That way, the calculations can be done based on the actual height of the textfield, even if it has been resized while the editor was disabled (resizing the hidden field in when the editor resizes is trickier since few of them seem to fire events for that).

It would make the patch smaller, and not require all profile settings to be duplicated for each field. That is still relevant for D7 because Drupal.settings grows quickly with the current approach and has to be regenerated on each page load.

Sorry, bumping this back to "Needs work", but I also promise I'll have a look at respinning the patch tonight (at work now).

TwoD’s picture

Status: Needs work » Needs review
FileSize
2.96 KB

These editors automatically adapt their height on attach:
jWysiwyg, markItUp, NicEdit, TinyMCE, Whizzywig, YUI.
Didn't verify if all of them actually provide height settings, but I don't think that really matters since they auto adjust.

These editors needed "manual" height adjustment:
CKEditor, EpicEditor, FCKeditor.
Some hardcoded height values in our implementation were removed but can be altered back in to override automatic adaptation.

Others:
WYMeditor: NOK; height completely controlled by skin. Forcibly override with inline styles on its iframe?
I only had v0.5-rc1, which is several years old, to test with. Looks like the project switched away from its old site and is now working from github and closing in on their 1.0.0 relase: https://github.com/wymeditor/wymeditor/releases
I completely lost track of this editor so I created #2273635: WYMeditor 1.0.0 release to deal with its newer versions instead.

openWysiwyg: Don't know, don't care, won't run, too old, looks dead, dropping it.

Now, I know the name of the issue says "width/height", but the patches and discussions so far have focused on just the height, which seems to be the main issue. This patch only adjusts the heights, but could easily extended to widths as well. From what I could tell, all editors appeared to use the textarea width though (or the width of its container), but I can add widths in too now, or later, if desired.

What do you think?

sun’s picture

Status: Needs review » Reviewed & tested by the community
+++ b/editors/js/fckeditor-2.6.js
@@ -4,6 +4,9 @@
 Drupal.wysiwyg.editor.attach.fckeditor = function(context, params, settings) {

I don't think we should patch/fix the FCKeditor integration anymore...

Ideally, it should be removed, but that cannot be done without bumping Wysiwyg's major version (because BC break).

However, IMO it would be fine to just leave it alone as "exists, but no longer really supported, because FCKeditor is CKEditor now".


This patch does not really resolve the actual/original feature request, but I also assume it resolves the actual, underlying problem space.

In case people really want support for an explicit height (disregarding the height of the textarea), let's create a separate issue for that.

  • Commit 61c7113 on 6.x-2.x by TwoD:
    - #507696 by hadsie, kndr, ksenzee, mcarbone, gaele, et al:...
  • Commit 6beb1b6 on 7.x-2.x by TwoD:
    - #507696 by hadsie, kndr, ksenzee, mcarbone, gaele, et al:...
TwoD’s picture

Status: Reviewed & tested by the community » Fixed
Related issues: +#2274177: Allow altering settings per editor and field

@sun, hah, I saw that one about explicit height per field coming! So, I decided this was a good time to post another patch I've been working on: #2274177: Allow altering settings per editor and field
It's a big patch, but it has huge benefits! :)

I agree about FCKeditor, but I don't mind keeping the implementation "alive" until a major version change. I know a few people are still using it and it's not been that difficult so far.

Pushed the patch to 7.x-2.x and 6.x-2.x, gave credit to the people who put most work into this from the beginning since they were so patient with me. ;)

One small note: This patch will also adjust heights for summary fields, but it's still not possible to set an explicit height for summary fields which is different from the main field. That can be added later though.

Status: Fixed » Closed (fixed)

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

pianomansam’s picture

Status: Closed (fixed) » Needs work

Issue #2274177: Allow altering settings per editor and field is still a work in progress and potentially fixes this, but that means this issue itself is still a work in progress. So I'm marking it as Needs work.

steinmb’s picture

Status: Needs work » Fixed

The rest is going to be tackled in #2274177: Allow altering settings per editor and field - Closing.

pianomansam’s picture

Status: Fixed » Closed (duplicate)

If #2274177: Allow altering settings per editor and field will fix this, better to make this a duplicate and track the other issue, because at this point in time, the issue is not fixed.