Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Make the CKEDITOR, FCKEditor, and TinyMCE editor heights respect the number of rows set in the field configuration.
Comment | File | Size | Author |
---|---|---|---|
#120 | wysiwyg_height.507696.120.patch | 2.96 KB | TwoD |
Comments
Comment #1
HasseMan CreditAttribution: HasseMan commentedLooking at wysiwyg.module this is what I find:
Hope that helps a bit.
Comment #2
sunWhy 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?
Comment #3
rconstantine CreditAttribution: rconstantine commentedWhy 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?
Comment #4
sunwell, multigroup or not - do you think that admins will (and want to) configure editors separately for every single field in their Drupal site?
Comment #5
rconstantine CreditAttribution: rconstantine commentedobviously, 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.
Comment #6
escoles CreditAttribution: escoles commentedConfiguring 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:
The first three need to be shorter than body copy fields, or you have the following problems with users:
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.
Comment #7
hadsie CreditAttribution: hadsie commentedJust 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
Comment #8
hadsie CreditAttribution: hadsie commentedAlso, 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?
Comment #9
hadsie CreditAttribution: hadsie commentedHere'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.
Comment #10
sunComment #11
drupalninja99 CreditAttribution: drupalninja99 commentedAny 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.
Comment #12
sunI 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?
Comment #13
drupalninja99 CreditAttribution: drupalninja99 commentedim 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
Comment #14
sunIf you ask that, then you are at least not using the latest version of Wysiwyg module. ;)
Comment #15
drupalninja99 CreditAttribution: drupalninja99 commentedI'm using version 6.x-2.1, what am I supposed to be using?
Comment #16
adeb CreditAttribution: adeb commentedSubscribing.
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.
Comment #17
HnLn CreditAttribution: HnLn commentedsubscribe (using ckeditor)
Comment #18
willieseabrook CreditAttribution: willieseabrook commentedI 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
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.
Comment #19
willieseabrook CreditAttribution: willieseabrook commentedThought about this more.
Would actually be smarter to append the $form object somehow to the $context, as that would deal with more situations.
Comment #20
HnLn CreditAttribution: HnLn commentedI agreee with #11, the editors heigth (and possibly width) should adjust to the nr of rows (cols) provided in cck.
Comment #21
kndr#9 works for me with TinyMCE editor. Thank you! I am attaching modified #9 patch for TinyMCE and FCKEeditor.
Comment #22
ksenzeeThis 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.
Comment #23
TwoDMaybe 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.)
Tab. ;)
No need to reroll for that.
Powered by Dreditor.
Comment #24
sun1) 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.
Comment #25
TwoD1) 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.
Comment #26
avner CreditAttribution: avner commentedsubscribing
Comment #27
Anonymous (not verified) CreditAttribution: Anonymous commentedThe 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?
Comment #28
mcarbone CreditAttribution: mcarbone commentedThis 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).
Comment #29
mcarbone CreditAttribution: mcarbone commentedOh, this is happening because of this function:
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.
Comment #30
mcarbone CreditAttribution: mcarbone commentedOK, 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.
Comment #31
emilyf CreditAttribution: emilyf commented@mcarbone, thanks!! initial testing with your #30 patch seems to be working great.
Comment #32
szantog CreditAttribution: szantog commentedThis patch in #30 worked with ckeditor.
Thank you!
Comment #33
3dloco CreditAttribution: 3dloco commented#30 works with ckeditor for me too. Thanks a lot!!!!
Comment #34
my-family CreditAttribution: my-family commented#30 works nice (tested for FCKeditor 2.6.6. so far), resolved my strong headache, thank you!!!
Comment #35
ayalon CreditAttribution: ayalon commented#30 <-- works perfekt!
This is the fourth test and considered to be rtbc.
I tested this with ckeditor and fckeditor.
Very good patch!
Comment #36
gooddesignusa CreditAttribution: gooddesignusa commentedpatch #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
Comment #37
sunThanks 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.
Comment #38
gooddesignusa CreditAttribution: gooddesignusa commentedIt 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.
Comment #39
szantog CreditAttribution: szantog commentedHmm, 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..
Comment #40
tallsimon CreditAttribution: tallsimon commentedFor 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
Comment #41
3dloco CreditAttribution: 3dloco commentedSeeing same issue mention in #39 (CKEditor disappear when user is previewing a node as a consequence of patch at #30)....:-(
Comment #42
mrwhizkid CreditAttribution: mrwhizkid commentedI 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.
Comment #43
Danny Englander#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.
Comment #44
Dave ReidSubscribe this is really useful. I'll work on re-rolling this for 7.x-2.x tonight.
Comment #45
TwoD@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...
Comment #46
sunI 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 introduceformat » field
overrides.Comment #47
Danny EnglanderIs there a way to tell if the patch has been rolled into the latest 6.x dev version?
Comment #48
TwoD@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.
Comment #49
Danny Englander@TwoD - thanks for the info. :)
Comment #50
Encarte CreditAttribution: Encarte commentedsubscribe
Comment #51
ayalon CreditAttribution: ayalon commentedI tried to fix the issues, but failed. We need some help from an experienced maintainer. #46 is not enough clear to me.
Comment #52
NPC CreditAttribution: NPC commentedThose 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):
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!
Comment #53
scott.whittaker CreditAttribution: scott.whittaker commentedSubscribe
Comment #54
yugongtian CreditAttribution: yugongtian commented+++
Comment #55
lightstring CreditAttribution: lightstring commentedSubscribing
Comment #56
ailgm CreditAttribution: ailgm commentedI'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?
Comment #57
Mark Vincent Verallo CreditAttribution: Mark Vincent Verallo commentedMy quick and dirty solution:
Change '100px' to your desired height.
Comment #58
AndyF CreditAttribution: AndyF commentedsubscribe
Comment #59
JustMagicMaria CreditAttribution: JustMagicMaria commentedSubscribe. (Using ckeditor and 7.x and could really use this feature.)
Comment #60
tchopshop CreditAttribution: tchopshop commentedSubscribe... for D7 and CKeditor
Comment #61
bforchhammer CreditAttribution: bforchhammer commentedDitto.
Comment #62
Danny Englander@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?
Comment #63
chichilatte CreditAttribution: chichilatte commentedYep, 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!
Comment #64
esbite CreditAttribution: esbite commentedPatch #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.
Comment #65
ksenzeeI 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.
Comment #66
drupal a11y CreditAttribution: drupal a11y commentedCheck 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
Comment #67
Anonymous (not verified) CreditAttribution: Anonymous commented#65 worked for me on 7.x-2.1. Note that you need to use the -p1 switch on the patch command i.e.
Thanks guys.
Comment #68
mpotter CreditAttribution: mpotter commentedThis patch (#65) worked for me and was VERY useful on my site!
(git apply works fine without any -p1 btw)
Comment #69
gaele CreditAttribution: gaele commentedThe patch from #65 does not work for textfields. Textfields have no row setting, though they will always be one row high.
Comment #70
gaele CreditAttribution: gaele commentedPatch 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.
Comment #71
Jason Dean CreditAttribution: Jason Dean commented#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.
Comment #72
David_Rothstein CreditAttribution: David_Rothstein commentedI 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.)
Comment #73
anouHello,
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.
Comment #74
mstef CreditAttribution: mstef commented#65 did a good job with the height..
Comment #75
dkingofpa CreditAttribution: dkingofpa commentedPatches 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 withgit 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.
Comment #76
sassafrass CreditAttribution: sassafrass commentedApplied patch in #64 and it worked perfectly, but had to apply it by hand. Using version = "6.x-2.4" with ckeditor. Thanks!
Comment #77
stevenovy CreditAttribution: stevenovy commentedApplied patch in #75 and field row settings for textarea fields are working great with CKEditor. Thanks all!!
Comment #78
loze CreditAttribution: loze commentedConfirming patch in #75 works for me. Thank You!
Comment #79
greggmarshallWhile 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.
Comment #80
samerali CreditAttribution: samerali commented#64 worked great, thanks!
Comment #81
ayalon CreditAttribution: ayalon commented#75
Confirming that #75 is working as expected and applies without error.
Comment #82
zhangtaihao CreditAttribution: zhangtaihao commented#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.
Comment #83
vegardjo CreditAttribution: vegardjo commented#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:
..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...
Comment #84
prinds CreditAttribution: prinds commentedWith 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?
Comment #85
prinds CreditAttribution: prinds commentedIt 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
Comment #86
gaele CreditAttribution: gaele commentedSo, #75 is working, except when using a Field Collection field in combination with the "Add another item" button.
I tested this:
Results:
Comment #87
brettbirschbach CreditAttribution: brettbirschbach commentedSubscribing.
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.
Comment #88
brettbirschbach CreditAttribution: brettbirschbach commentedSo 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.
Comment #89
brettbirschbach CreditAttribution: brettbirschbach commentedAdding 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.
Comment #90
Anonymous (not verified) CreditAttribution: Anonymous commentedthe patch from #75 is no longer working with the latest dev (2013-Feb-23)
Comment #91
gaele CreditAttribution: gaele commentedComment #92
Grayside CreditAttribution: Grayside commented#75 still works for 7.x-2.2
Comment #93
pedrospFor 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.
Comment #94
mstrelan CreditAttribution: mstrelan commented#75 is great, although not compatible with #741606-45: Teaser splitter / text fields with summary support.
Comment #95
mstrelan CreditAttribution: mstrelan commentedAs 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.
Comment #96
WorldFallz CreditAttribution: WorldFallz commented#75 no longer applies-- here's an straight reroll against the current dev.
Comment #97
ShaunDychko CreditAttribution: ShaunDychko commented#96 works great. Thank you.
Comment #98
ShaunDychko CreditAttribution: ShaunDychko commentedComment #99
Anonymous (not verified) CreditAttribution: Anonymous commentedThere seems to be a problem with #96 with current dev. There is no wysiwyg.js file in the js/ directory
Comment #100
Anonymous (not verified) CreditAttribution: Anonymous commentedAlso, changes listed in #96 to wysiwyg.module cause WSOD.
Comment #101
juliusvaart CreditAttribution: juliusvaart commented#96 works great. Also on Field Collections.
Comment #102
czigor CreditAttribution: czigor commented@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.
Comment #103
anouI can confirm that patch #96 works great, even with my module's custom fields! ;-)
Comment #104
Anonymous (not verified) CreditAttribution: Anonymous commentedAnyone 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.
Comment #105
ayalon CreditAttribution: ayalon commentedI mage a patch against the current dev:
https://drupal.org/files/issues/wysiwyg_field_size_507696_updated.patch
Comment #106
ayalon CreditAttribution: ayalon commentedComment #107
jenlamptonIt 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
Comment #108
broeker CreditAttribution: broeker commentedUpdate: 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).
Comment #109
bneil CreditAttribution: bneil commentedUpdating to needs work per #108
Comment #110
Anonymous (not verified) CreditAttribution: Anonymous commentedRe-rolled against latest dev.
Comment #111
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #112
Anonymous (not verified) CreditAttribution: Anonymous commentedFixed last patch - missed one line when re-rolling patch.
Comment #113
hwasem CreditAttribution: hwasem commentedI tested #112 against latest dev (Wysiwyg 7.x-2.2+33-dev (2014-Mar-19)) and it failed on hunk#2 at 471.
Comment #114
Anonymous (not verified) CreditAttribution: Anonymous commentedHmm, weird. Re-rolled again and tested against 33-dev. Applied clean.
Comment #115
hwasem CreditAttribution: hwasem commentedThat 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!
Comment #116
Vacilando CreditAttribution: Vacilando commentedApplies 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.
Comment #117
rjacobs CreditAttribution: rjacobs commentedThis 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?
Comment #118
rjacobs CreditAttribution: rjacobs commentedHummmm, 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.
Comment #119
TwoDHmm, 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).
Comment #120
TwoDThese 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?
Comment #121
sunI 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.
Comment #123
TwoD@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.
Comment #125
pianomansam CreditAttribution: pianomansam commentedIssue #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.
Comment #126
steinmb CreditAttribution: steinmb as a volunteer and at University Of Bergen commentedThe rest is going to be tackled in #2274177: Allow altering settings per editor and field - Closing.
Comment #127
pianomansam CreditAttribution: pianomansam commentedIf #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.