Bug 87654

Summary: [EFL] Refactoring. Get rid of unnecessary singleCharacterString() function
Product: WebKit Reporter: Mikhail Pozdnyakov <mikhail.pozdnyakov>
Component: WebKit EFLAssignee: Mikhail Pozdnyakov <mikhail.pozdnyakov>
Status: RESOLVED FIXED    
Severity: Normal CC: gyuyoung.kim, lucas.de.marchi, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
none
patch v2 (Review remarks are met) none

Mikhail Pozdnyakov
Reported 2012-05-28 06:10:33 PDT
Evas key events already provide key string, for instance if the User presses 'Return' key the generated Evas_Event_Key_Down(_Up) events will contain following fields: keyname = "Return", key = "Return", string = "\r", compose = "\r" So no practical need for singleCharacterString() evaluating the event string from event keyname. The only place where this function is currently used is DRT, however DRT EventSender also can provide event string itself.
Attachments
patch (12.41 KB, patch)
2012-05-28 06:25 PDT, Mikhail Pozdnyakov 		no flags
DetailsFormatted DiffDiff
patch v2 (Review remarks are met) (12.86 KB, patch)
2012-05-28 07:05 PDT, Mikhail Pozdnyakov 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Mikhail Pozdnyakov
Comment 1 2012-05-28 06:25:25 PDT
Created attachment 144345 [details] patch
Raphael Kubo da Costa (:rakuco)
Comment 2 2012-05-28 06:34:30 PDT
Comment on attachment 144345 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=144345&action=review > Tools/DumpRenderTree/efl/EventSender.cpp:124 > + KeyEventInfo(const CString& keyName, unsigned modifiers) > + : keyName(keyName) > + , modifiers(modifiers) Instead of adding a new constructor, I'd change the order of parameters in the other one and make keyString an optional last parameter whose default value is an empty string.
Mikhail Pozdnyakov
Comment 3 2012-05-28 07:05:29 PDT
Created attachment 144353 [details] patch v2 (Review remarks are met)
Mikhail Pozdnyakov
Comment 4 2012-05-28 07:06:40 PDT
(In reply to comment #2) > (From update of attachment 144345 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=144345&action=review > > > Tools/DumpRenderTree/efl/EventSender.cpp:124 > > + KeyEventInfo(const CString& keyName, unsigned modifiers) > > + : keyName(keyName) > > + , modifiers(modifiers) > > Instead of adding a new constructor, I'd change the order of parameters in the other one and make keyString an optional last parameter whose default value is an empty string. done.
Raphael Kubo da Costa (:rakuco)
Comment 5 2012-05-28 07:19:52 PDT
Comment on attachment 144353 [details] patch v2 (Review remarks are met) Looks good, thanks.
Gyuyoung Kim
Comment 6 2012-05-28 18:14:30 PDT
Comment on attachment 144353 [details] patch v2 (Review remarks are met) Looks good to me too.
Csaba Osztrogonác
Comment 7 2012-05-29 04:48:34 PDT
Comment on attachment 144353 [details] patch v2 (Review remarks are met) rs=me
WebKit Review Bot
Comment 8 2012-05-29 05:13:18 PDT
Comment on attachment 144353 [details] patch v2 (Review remarks are met) Clearing flags on attachment: 144353 Committed r118760: <http://trac.webkit.org/changeset/118760>
WebKit Review Bot
Comment 9 2012-05-29 05:13:22 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.