Bug 83797 – [EFL] [DRT] Feeding key events with invalid keyName

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Sudarsana Nagineni (babu)]

keyName passed to KeyEventInfo is not valid after this function returns, so use CString to keep the keyName valid within the scope of KeyEventInfo.

static KeyEventInfo* createKeyEventInfo(JSContextRef context, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
{
....
const CString cCharacter = character.get()->ustring().utf8();
....
if (!keyName)
     keyName = cCharacter.data();

return new KeyEventInfo(keyName, modifiers);
}

[Sudarsana Nagineni (babu)]

Created attachment 136942 [details]
Patch

Using CString to keep the keyName valid within scope of KeyEventInfo

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 136942 [details]
Patch

Makes sense, thanks.

[Philippe Normand]

Comment on attachment 136942 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=136942&action=review

> Tools/DumpRenderTree/efl/EventSender.cpp:437
>      const CString cCharacter = character.get()->ustring().utf8();
>      const char* keyName = (location == DomKeyLocationNumpad) ? keyPadNameFromJSValue(character.get()) : keyNameFromJSValue(character.get());
> -    if (!keyName)
> -        keyName = cCharacter.data();
>  
> -    return new KeyEventInfo(keyName, modifiers);
> +    return new KeyEventInfo(keyName ? CString(keyName) : cCharacter, modifiers);

Seems like keyPadNameFromJSValue() should return a CString now. That would simplify this code block, I think.

[Sudarsana Nagineni (babu)]

(In reply to comment #3)
> (From update of attachment 136942 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=136942&action=review
> 
> > Tools/DumpRenderTree/efl/EventSender.cpp:437
> >      const CString cCharacter = character.get()->ustring().utf8();
> >      const char* keyName = (location == DomKeyLocationNumpad) ? keyPadNameFromJSValue(character.get()) : keyNameFromJSValue(character.get());
> > -    if (!keyName)
> > -        keyName = cCharacter.data();
> >  
> > -    return new KeyEventInfo(keyName, modifiers);
> > +    return new KeyEventInfo(keyName ? CString(keyName) : cCharacter, modifiers);
> 
> Seems like keyPadNameFromJSValue() should return a CString now. That would simplify this code block, I think.

Thanks for your review Philippe.
Yes, it simplifies the code. I will update the patch.

[Sudarsana Nagineni (babu)]

Created attachment 137335 [details]
Patch

fixed review comment #3.

[WebKit Review Bot]

Comment on attachment 137335 [details]
Patch

Clearing flags on attachment: 137335

Committed r114250: <http://trac.webkit.org/changeset/114250>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.