openModalInsert allows fields with a name attribute when building the html, but ignores it when interpreting the result

[nikkilocke]

Informations

Browser version: Google Chrome | 63.0.3239.132 (Official Build) (64-bit) (cohort: Stable)
OS: Windows 10
Screen resolution:1080x1920

How to reproduce the bug?

I am trying to use radio buttons in an openModalInsert. I know this is not supported, but the bug applies to any field where a name attribute is set.

			imgDblClickHandler: function () {
				var t = i.data('trumbowyg'),
					$img = $(this),
					src = $img.attr('src'),
					base64 = '(Base64)',
					style = $img.attr('style') || '';

				if (src.indexOf('data:image') === 0) {
					src = base64;
				}

				var options = {
					url: {
						label: 'URL',
						value: src,
						required: true
					},
					alt: {
						label: t.lang.description,
						value: $img.attr('alt')
					},
					noFloat: {
						label: 'No Float',
						type: 'radio',
						name: 'floatImage',    // **** Same name to force radio group
						value: ''
					},
					floatLeft: {
						label: 'Float left',
						type: 'radio',
						name: 'floatImage',    // **** Same name to force radio group
						value: 'float:left'
					},
					floatRight: {
						label: 'Float image to right',
						type: 'radio',
						name: 'floatImage',    // **** Same name to force radio group
						value: 'float:right'
					}
				};
				if(style.search('left') >= 0)
					options.floatLeft.attributes = { checked: true };
				 else if(style.search('right') >= 0)
					options.floatRight.attributes = { checked: true };
				else
					options.noFloat.attributes = { checked: true };
				if (t.o.imageWidthModalEdit) {
					options.width = {
						value: $img.attr('width') ? $img.attr('width') : ''
					};
				}

				t.openModalInsert(t.lang.insertImage, options, function (v) {
				return false;
			}

openModalInsert has this code:

        $.each(fields, function (fieldName, field) {
            var l = field.label,
                n = field.name || fieldName,    // **** Note - name attribute is explicitly catered for
                a = field.attributes || {};

            var attr = Object.keys(a).map(function (prop) {
                return prop + '="' + a[prop] + '"';
            }).join(' ');

            html += '<label><input type="' + (field.type || 'text') + '" name="' + n + '" value="' + (field.value || '').replace(/"/g, '&quot;') + '"' + attr + '><span class="' + prefix + 'input-infos"><span>' +
                ((!l) ? (lg[fieldName] ? lg[fieldName] : fieldName) : (lg[l] ? lg[l] : l)) +
                '</span></span></label>';
        });

It also has an on event handler:

        return t.openModal(title, html)
            .on(CONFIRM_EVENT, function () {
                var $form = $('form', $(this)),
                    valid = true,
                    values = {};

                $.each(fields, function (fieldName, field) {
                    var $field = $('input[name="' + fieldName + '"]', $form),
                        inputType = $field.attr('type');

                    if (inputType.toLowerCase() === 'checkbox') {

Note that in the event handler, it takes no account of the name attribute of the field, so $field is empty, and inputType is undefined, so toLowerCase throws an exception.

The code should either ignore the name attribute when building the html, or honour it when reading the values back. Code for the latter would be:

	var n = field.name || fieldName;
            var $field = $('input[name="' + n + '"]', $form),

The code could be altered to handle radio buttons correctly by changing:

                    if (inputType.toLowerCase() === 'checkbox') {
                        values[fieldName] = $field.is(':checked');
                    } else {
                        values[fieldName] = $.trim($field.val());
                    }

to:

                    switch (inputType.toLowerCase()) {
                    case 'checkbox':
                        values[n] = $field.is(':checked');
                        break;
                    case 'radio':
                        values[n] = $field.filter(':checked').val();
                        break;
                    default:
                        values[n] = $.trim($field.val());
                        break;
                    }

If you wish I will create a pull request, but I am unsure whether you will feel the extra 7 lines of code would be useful enough.

[Alex-D]

You can make PR if you want, it's more readable that code droped here ^^

If it's light, it can come as a core feature.

[nikkilocke]

OK. I am not sure how you update the files in dist, particularly the .min.js files. Is there a script I should run to generate the .min.js files? Or should I just check in the src changes, and leave the dist to you?

[nikkilocke]

Likewise the documentation - I am adding a few lines to docs/documentation/core/index.html to explain the use of type and attributes in openModalInsert fields, but I imagine docs/index.html gets updated by a script somewhere?

[Alex-D]

min files are only into master, generated at release time by me.

docs/index.html is the home here: https://alex-d.github.io/Trumbowyg/ so I did not understand which update are you talking about :/

[Alex-D]

Merged :)

In develop for now.