Opened 13 years ago

Closed 12 years ago

#9019 closed Bug (fixed)

jQuery Adapter .val() replaced handles .val(undefined) inconsistently

Reported by: Joel Howard Owned by: Piotr Jasiun
Priority: Normal Milestone: CKEditor 4.2
Component: General Version: 3.1
Keywords: Cc:

Description

The jQuery Adapter version of jQuery.val() does not handle .val(undefined) the same way the original jQuery.val() does. jQuery.val() uses the arguments.length property to determine whether the .val() call is a 'get' or a 'set', which determines what the function will return (the element's value or its jQuery object, respectively). This is done in the following code (from jQuery 1.7.1, attributes.js).

val: function( value ) {
		var hooks, ret, isFunction,
			elem = this[0];

		if ( !arguments.length ) {
			if ( elem ) {
				hooks = jQuery.valHooks[ elem.type ] || jQuery.valHooks[ elem.nodeName.toLowerCase() ];

				if ( hooks && "get" in hooks && (ret = hooks.get( elem, "value" )) !== undefined ) {
					return ret;
				}

				ret = elem.value;

				return typeof ret === "string" ?
					// handle most common string cases
					ret.replace(rreturn, "") :
					// handle cases where value is null/undef or number
					ret == null ? "" : ret;
			}

			return;
		}

		isFunction = jQuery.isFunction( value );

		return this.each(function( i ) {
			var self = jQuery(this), val;

			if ( this.nodeType !== 1 ) {
				return;
			}

			if ( isFunction ) {
				val = value.call( this, i, self.val() );
			} else {
				val = value;
			}

			// Treat null/undefined as ""; convert numbers to string
			if ( val == null ) {
				val = "";
			} else if ( typeof val === "number" ) {
				val += "";
			} else if ( jQuery.isArray( val ) ) {
				val = jQuery.map(val, function ( value ) {
					return value == null ? "" : value + "";
				});
			}

			hooks = jQuery.valHooks[ this.type ] || jQuery.valHooks[ this.nodeName.toLowerCase() ];

			// If set returns undefined, fall back to normal setting
			if ( !hooks || !("set" in hooks) || hooks.set( this, val, "value" ) === undefined ) {
				this.value = val;
			}
		});
	}

The jQuery adapter works differently, explicitly checking the first argument for undefined instead of using arguments.length:

			return function( newValue, forceNative )
			{
				var isSetter = typeof newValue != 'undefined',
					result;

				this.each( function()
				{
					var $this = jQuery( this ),
						editor = $this.data( 'ckeditorInstance' );

					if ( !forceNative && $this.is( 'textarea' ) && editor )
					{
						if ( isSetter )
							editor.setData( newValue );
						else
						{
							result = editor.getData();
							// break;
							return null;
						}
					}
					else
					{
						if ( isSetter )
							oldValMethod.call( $this, newValue );
						else
						{
							result = oldValMethod.call( $this );
							// break;
							return null;
						}
					}

					return true;
				});
				return isSetter ? this : result;
			};

This results in inconsistency when .val(undefined) is called. jQuery treats this as a setter and returns the jQuery object, whereas the jQuery Adapter treats this as a getter and returns the object's value. This breaks chaining, and probably has other consequences.

Change History (6)

comment:1 Changed 13 years ago by Jakub Ś

Keywords: jquery adapter removed
Status: newconfirmed
Version: 3.1

Reproducible from CKE 3.1

comment:2 Changed 12 years ago by Piotr Jasiun

Owner: set to Piotr Jasiun
Status: confirmedreview

comment:3 Changed 12 years ago by Piotr Jasiun

Milestone: CKEditor 4.2

comment:4 Changed 12 years ago by Piotr Jasiun

in branch/ticket #10281

comment:5 Changed 12 years ago by Piotr Jasiun

Status: reviewassigned

comment:6 Changed 12 years ago by Olek Nowodziński

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy