Dialog function clearOrRecoverTextInputValue() removes data from text input elements

[Arve Skjørestad]

We have a custom image insert dialog, where we populate the data in the "txtUrl" field (a regular input type text) with data from an external service, loaded using an Ajax request/callback. We then hides the element from the GUI (the user sees another selector, not the "raw image url".

After upgrading to 3.6.2, image edit in IE7 stopped working, and we found that the value in txtUrl was cleared (we could see it having the correct value, and then beeing cleared).

After a lot of debugging, we found that the method clearOrRecoverTextInputValue() in plugins/dialog/plugin.js was responsible for clearing the data.

(Since reproducing this bug involves setting up a custom dialog and and Ajax service I am not writing steps to reproduce here, but instead an explanation and a suggestion to code fix)

In the first run of clearOrRecoverTextInputValue(), the method saves the value of the input element into a custom attribute named "fake_value", and then sets the element's real value to . Since the value of our element at this stage is empty, is saved as value of the "fake_value" attribute.

Now, in between here our Ajax callback runs and returns, and populates the value of the input element with an URL (using the official API methods).

Then, in the second run, clearOrRecoverTextInputValue() attempts to restore the value saved in the attribute "fake_value" into the element's real value attribute. The value of this is still , and therefore is set as the real value of the element, overwriting what has been set from our Ajax callback.

clearOrRecoverTextInputValue() should only restore the value from "fake_value" into the elements real value if that is , any other real values means that someone else has modified the value in between the two calls to clearOrRecoverTextInputValue(), and that the elementæs real value should remain unchanged.

Code suggestion:

change: if ( isRecover )

{

item.setAttribute( 'value', item.getCustomData( 'fake_value' )

);

item.removeCustomData( 'fake_value' );

into: if ( isRecover ) {

if (item.getAttribute( 'value' ) == ) {

item.setAttribute( 'value', item.getCustomData( 'fake_value' )

);

} item.removeCustomData( 'fake_value' );

[Arve Skjørestad]

Arg, the ticket system obviously strips away two single quotes following each other, which makes my description slightly less meaningless...

Trying again, this time with double quotes:

We have a custom image insert dialog, where we populate the data in the "txtUrl" field (a regular input type text) with data from an external service, loaded using an Ajax request/callback. We then hides the element from the GUI (the user sees another selector, not the "raw image url".

After upgrading to 3.6.2, image edit in IE7 stopped working, and we found that the value in txtUrl was cleared (we could see it having the correct value, and then beeing cleared).

After a lot of debugging, we found that the method clearOrRecoverTextInputValue() in plugins/dialog/plugin.js was responsible for clearing the data.

(Since reproducing this bug involves setting up a custom dialog and and Ajax service I am not writing steps to reproduce here, but instead an explanation and a suggestion to code fix)

In the first run of clearOrRecoverTextInputValue(), the method saves the value of the input element into a custom attribute named "fake_value", and then sets the element's real value to "" (empty string) . Since the value of our element at this stage is empty, "" (empty string) is saved as value of the "fake_value" attribute.

Now, in between here our Ajax callback runs and returns, and populates the value of the input element with an URL (using the official API methods).

Then, in the second run, clearOrRecoverTextInputValue() attempts to restore the value saved in the attribute "fake_value" into the element's real value attribute. The value of this is still "" (empty string), and therefore is set as the real value of the element, overwriting what has been set from our Ajax callback.

clearOrRecoverTextInputValue() should only restore the value from "fake_value" into the elements real value if that is , any other real values means that someone else has modified the value in between the two calls to clearOrRecoverTextInputValue(), and that the element's real value should remain unchanged.

Code suggestion:

change: if ( isRecover ) {

item.setAttribute( 'value', item.getCustomData( 'fake_value' )); item.removeCustomData( 'fake_value' );

into: if ( isRecover ) {

if (item.getAttribute( 'value' ) == "") {

item.setAttribute( 'value', item.getCustomData( 'fake_value' ));

} item.removeCustomData( 'fake_value' );

[Jakub Ś]

In the first run of clearOrRecoverTextInputValue(), the method saves the value of the input element into a custom attribute named "fake_value", and then sets the element's real value to "" (empty string) . Since the value of our element at this stage is empty, "" (empty string) is saved as value of the "fake_value" attribute.

I see you have found workaround for it - that's fine as it can be used as one solution.

I see that you have made some custom chnage which is using part of our code. Why can't you update "fake_value"? As you have noticed it, it populates value at later time so perhaps the best idea would to have your plugin change it? This could be your second solution.

We are making changes to fix things that we know are broken. You must agree that it is impossible to foresee that those changes might brake some custom plugin and to be honest it will be rather impossible to fix it now especially that we know nothing about this plugin.

Since there is no TC, no steps to reproduce and issue concerns custom change I will close this issue as invalid. Please leave a comment if you don't agree and think it should be reopened or have anything else to add.