116303 - JavaScript prompt loses default input text

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Tim Powell]

The JavaScript window.prompt function takes two parameters. The first is for the
message to be displayed in the dialog about what you want the user to enter and
the second is default input text. This default text is now being lost. I noticed
this when using a bugzilla bookmarket and verified it by entering the javascript
prompt directly into the URL box.

To reproduce, try the URL above.
javascript:prompt('Do you see any default text in the input box?','Yes');

Expected:
You should see Yes in the input box. Technically it should also be selected so
that typing will erase all that's there, but that's bug 28583 and been a problem
for a long time.

Actual:
You see nothing in the input box. No default text is there.

I saw this using 20011219 11 build on win2k. This was not a problem with
20011217 09 build.

Comment 1

[Håkan Waara]

wfm. 2001-12-24-08, Windows 98.

Comment 2

[Tim Powell]

Reopening. This does not work with the 20011228 build on Win2K. No text appears
in the input box.

Comment 3

[Basil Fritts]

WFM 2002011506-trunk on windows 98. Closing after consult w/ reporter.

Comment 4

[Tim Powell]

It appears that this bug is caused as a side effect of the pref for remembering
passwords. To reporoduce, go to Edit menu, Preferences, expand the Privacy &
Security tree, pick Web Passwords, and then uncheck the box for Remember
passwords for sites that require me to log in. It will now consistently fail
(the default text does not appear). Go back and check the box again and it works
as expected.

This behavior should not be tied to the passwords pref. JavaScript prompt should
show default text no matter what. This breaks a number of my bookmarklets and
the workaround (enabling the pref) is not at all intuitive. The workaround is
also unacceptable for security reasons.

Comment 5

[Tim Powell]

Should have mentioned that I saw this problem with the 2002012309 build on
Win2K. It is completely reproducable.

Comment 6

[Blake Ross]

-> morse?

Comment 7

[Stephen P. Morse]

Attachment: put up default text even if pm pref is off

Comment 8

[Stephen P. Morse]

alecf, sgehani, please review.  Thanks.

Comment 9

[Alec Flett]

are we going to leak if the user actually enters some text? this doesn't look right.

Comment 10

[Stephen P. Morse]

The dialog->prompt statement in singsign.cpp calls nsPrompt::Prompt which in 
turn calls nsPromptService::Prompt.  That gets us into nsPromptService.cpp with 
the following code:

  rv = DoDialog(parent, block, kPromptURL);
  if (NS_FAILED(rv))
    return rv;

  PRInt32 tempInt = 0;
  block->GetInt(eButtonPressed, &tempInt);
  *_retval = tempInt ? PR_FALSE : PR_TRUE;

  if (*_retval) {
    PRUnichar *tempStr;
    
    rv = block->GetString(eEditfield1Value, &tempStr);
    if (NS_FAILED(rv))
      return rv;
    if (*value)
      nsMemory::Free(*value);
    *value = tempStr;

    if (checkValue)
      block->GetInt(eCheckboxState, checkValue);  
  }

That's where the memory gets freed.  At least as far as the resultText that we 
allocated in singsign.cpp before calling on the dialog.  Of course there's now a 
new resultText which the dialog is returning and that gets freed up by the 
caller of the routine in singsign.cpp.

Comment 11

[Samir Gehani]

Comment on attachment 67038 [details] [diff] [review]
put up default text even if pm pref is off

r=sgehani  
Thanks for explaining.

Comment 12

[Alec Flett]

Comment on attachment 67038 [details] [diff] [review]
put up default text even if pm pref is off

yikes! I think that's really broken. (nsPromptService that is) - the parameter
is
designated as "inout" which means that when taken as an "in" the parameter
should not be freed by the callee, but the callee should allocate a new
result...

i.e. nsPromptService should not be freeing "value", and your code SHOULD say:

*resultText = ToNewUnicode(data);
char *temp = *resultText;
dialog->Prompt(..., &val, ...);
nsMemory::Free(temp);
nsMemory::Free(*resultText); // or maybe this gets passed on elswhere

however I could be wrong here. 
I'm cc'ing jband for ownership issues on inout parameters.

in the mean time, sr=alecf on this patch.

Comment 13

[Stephen P. Morse]

checked in.