From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by mx.groups.io with SMTP id smtpd.web11.8660.1582095319119733875 for ; Tue, 18 Feb 2020 22:55:19 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.151, mailfrom: zhichao.gao@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Feb 2020 22:55:18 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,459,1574150400"; d="scan'208";a="229014204" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga008.jf.intel.com with ESMTP; 18 Feb 2020 22:55:18 -0800 Received: from shsmsx602.ccr.corp.intel.com (10.109.6.142) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 18 Feb 2020 22:55:17 -0800 Received: from shsmsx603.ccr.corp.intel.com (10.109.6.143) by SHSMSX602.ccr.corp.intel.com (10.109.6.142) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Wed, 19 Feb 2020 14:55:15 +0800 Received: from shsmsx603.ccr.corp.intel.com ([10.109.6.143]) by SHSMSX603.ccr.corp.intel.com ([10.109.6.143]) with mapi id 15.01.1713.004; Wed, 19 Feb 2020 14:55:15 +0800 From: "Gao, Zhichao" To: "devel@edk2.groups.io" , "vit9696@protonmail.com" Subject: Re: [edk2-devel] [PATCH 1/1] ShellPkg: Add support for input with separately reported modifiers Thread-Topic: [edk2-devel] [PATCH 1/1] ShellPkg: Add support for input with separately reported modifiers Thread-Index: AQHV3/vd2AwDJwjHrU+qLrlCVvdzV6giIlNg Date: Wed, 19 Feb 2020 06:55:15 +0000 Message-ID: References: <20200210101811.18741-1-vit9696@protonmail.com> <20200210101811.18741-2-vit9696@protonmail.com> In-Reply-To: <20200210101811.18741-2-vit9696@protonmail.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiOGQwYzA3ZDYtZTZkMy00MDFmLWI4MjgtNzQwZjRmNTZkNzg2IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiZVlLcEx3VEdocThkcTgyNHA3dGxpcUE3UWF2cXlSSnJvS0N2RWFvTlV4ZlBIN3VYUGVcL1Y4cjdYY3NhRGlaTXUifQ== dlp-reaction: no-action dlp-version: 11.2.0.6 x-originating-ip: [10.239.127.36] MIME-Version: 1.0 Return-Path: zhichao.gao@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Vitaly, See the comment below: > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Vitaly > Cheptsov via Groups.Io > Sent: Monday, February 10, 2020 6:18 PM > To: devel@edk2.groups.io > Subject: [edk2-devel] [PATCH 1/1] ShellPkg: Add support for input with > separately reported modifiers >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2510 >=20 > Some firmwares: > - Report Shift modifier even when they report upper-case unicode letter. > - Report Ctrl modifier with "shifted" UniChar (i.e. X - 'A' + 1). >=20 > This change provides support for these firmwares preserving the compatibi= lity > with the previous input handling. >=20 > Signed-off-by: Michael Belyaev > Reviewed-by: Vitaly Cheptsov > --- > ShellPkg/Library/UefiShellDebug1CommandsLib/Edit/MainTextEditor.c | 37 > ++++++++++++++------ > ShellPkg/Library/UefiShellDebug1CommandsLib/EditInputBar.c | 28 > ++++++++++----- > ShellPkg/Library/UefiShellDebug1CommandsLib/EditMenuBar.c | 6= ++++ > ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/MainHexEditor.c | 11 > +++--- > 4 files changed, 58 insertions(+), 24 deletions(-) >=20 > diff --git > a/ShellPkg/Library/UefiShellDebug1CommandsLib/Edit/MainTextEditor.c > b/ShellPkg/Library/UefiShellDebug1CommandsLib/Edit/MainTextEditor.c > index df530f1119..9615a4dfbd 100644 > --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Edit/MainTextEditor.c > +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Edit/MainTextEditor.c > @@ -1381,8 +1381,8 @@ MainCommandDisplayHelp ( > continue; > } >=20 > - if (((KeyData.KeyState.KeyShiftState & EFI_SHIFT_STATE_VALID) =3D=3D= 0) || > - (KeyData.KeyState.KeyShiftState =3D=3D EFI_SHIFT_STATE_VALID)) { > + if (((KeyData.KeyState.KeyShiftState & EFI_SHIFT_STATE_VALID) =3D=3D= 0) > + || (KeyData.KeyState.KeyShiftState =3D=3D EFI_SHIFT_STATE_VALID)) = { > // > // For consoles that don't support/report shift state, > // CTRL+W is translated to L'W' - L'A' + 1. > @@ -1390,14 +1390,17 @@ MainCommandDisplayHelp ( > if (KeyData.Key.UnicodeChar =3D=3D L'W' - L'A' + 1) { > break; > } > - } else if (((KeyData.KeyState.KeyShiftState & EFI_SHIFT_STATE_VALID)= !=3D 0) > && > - ((KeyData.KeyState.KeyShiftState & (EFI_LEFT_CONTROL_PRES= SED | > EFI_RIGHT_CONTROL_PRESSED)) !=3D 0) && > - ((KeyData.KeyState.KeyShiftState & ~(EFI_SHIFT_STATE_VALI= D | > EFI_LEFT_CONTROL_PRESSED | EFI_RIGHT_CONTROL_PRESSED)) =3D=3D 0)) { > + } else if (((KeyData.KeyState.KeyShiftState & EFI_SHIFT_STATE_VALID)= !=3D 0) > + && ((KeyData.KeyState.KeyShiftState & (EFI_LEFT_CONTROL_PRES= SED | > EFI_RIGHT_CONTROL_PRESSED)) !=3D 0) > + && ((KeyData.KeyState.KeyShiftState & > + ~(EFI_SHIFT_STATE_VALID | EFI_LEFT_CONTROL_PRESSED | > + EFI_RIGHT_CONTROL_PRESSED)) =3D=3D 0)) { > // > // For consoles that supports/reports shift state, > // make sure that only CONTROL shift key is pressed. > + // For some consoles that report shift state, > + // CTRL+W is still translated to L'W' - L'A' + 1. > // > - if ((KeyData.Key.UnicodeChar =3D=3D 'w') || (KeyData.Key.UnicodeCh= ar =3D=3D 'W')) > { > + if ((KeyData.Key.UnicodeChar =3D=3D L'w') || (KeyData.Key.UnicodeC= har =3D=3D L'W') > + || (KeyData.Key.UnicodeChar =3D=3D L'w' - L'a' + 1) || > + (KeyData.Key.UnicodeChar =3D=3D L'W' - L'A' + 1)) { > break; > } > } > @@ -1834,7 +1837,8 @@ MainEditorKeyInput ( > EFI_KEY_DATA KeyData; > EFI_STATUS Status; > EFI_SIMPLE_POINTER_STATE MouseState; > - BOOLEAN NoShiftState; > + BOOLEAN NoModifierState; > + BOOLEAN ShiftOnlyState; >=20 > do { >=20 > @@ -1886,17 +1890,28 @@ MainEditorKeyInput ( > // > StatusBarSetRefresh(); > // > - // NoShiftState: TRUE when no shift key is pressed. > + // NoModifierState: TRUE when no modifier key is pressed. > // > - NoShiftState =3D ((KeyData.KeyState.KeyShiftState & > EFI_SHIFT_STATE_VALID) =3D=3D 0) || (KeyData.KeyState.KeyShiftState =3D= =3D > EFI_SHIFT_STATE_VALID); > + NoModifierState =3D ((KeyData.KeyState.KeyShiftState & > EFI_SHIFT_STATE_VALID) =3D=3D 0) > + || (KeyData.KeyState.KeyShiftState =3D=3D EFI_SH= IFT_STATE_VALID); > + // > + // ShiftOnlyState: TRUE when no modifier key except Shift is pre= ssed. > + // > + ShiftOnlyState =3D ((KeyData.KeyState.KeyShiftState & > EFI_SHIFT_STATE_VALID) =3D=3D 0) > + || ((KeyData.KeyState.KeyShiftState > + & ~(EFI_SHIFT_STATE_VALID | > + EFI_LEFT_SHIFT_PRESSED | EFI_RIGHT_SHIFT_PRESSED)) =3D=3D 0); > // > // dispatch to different components' key handling function > // > if (EFI_NOT_FOUND !=3D MenuBarDispatchControlHotKey(&KeyData)) { > Status =3D EFI_SUCCESS; > - } else if (NoShiftState && ((KeyData.Key.ScanCode =3D=3D SCAN_NU= LL) || > ((KeyData.Key.ScanCode >=3D SCAN_UP) && (KeyData.Key.ScanCode <=3D > SCAN_PAGE_DOWN)))) { > + } else if ((ShiftOnlyState && (KeyData.Key.ScanCode =3D=3D SCAN_= NULL)) > + || (NoModifierState && (KeyData.Key.ScanCode >=3D SCAN_U= P) && > (KeyData.Key.ScanCode <=3D SCAN_PAGE_DOWN))) { > + // > + // alphanumeric keys with or without shift, or arrow keys with= out shift > + // This is unmatched with the comments. It only handles the alphanumeric keys = with shift. Thanks, Zhichao > Status =3D FileBufferHandleInput (&KeyData.Key); > - } else if (NoShiftState && (KeyData.Key.ScanCode >=3D SCAN_F1) &= & > (KeyData.Key.ScanCode <=3D SCAN_F12)) { > + } else if (NoModifierState && (KeyData.Key.ScanCode >=3D SCAN_F1= ) > + && (KeyData.Key.ScanCode <=3D SCAN_F12)) { > Status =3D MenuBarDispatchFunctionKey (&KeyData.Key); > } else { > StatusBarSetStatusString (L"Unknown Command"); diff --git > a/ShellPkg/Library/UefiShellDebug1CommandsLib/EditInputBar.c > b/ShellPkg/Library/UefiShellDebug1CommandsLib/EditInputBar.c > index 35b0b701e8..d053059220 100644 > --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/EditInputBar.c > +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/EditInputBar.c > @@ -130,6 +130,8 @@ InputBarRefresh ( > UINTN EventIndex; > UINTN CursorRow; > UINTN CursorCol; > + BOOLEAN ShiftPressed; > + BOOLEAN ModifiersPressed; >=20 > // > // variable initialization > @@ -180,17 +182,23 @@ InputBarRefresh ( > if (EFI_ERROR (Status)) { > continue; > } > - if (((KeyData.KeyState.KeyShiftState & EFI_SHIFT_STATE_VALID) !=3D 0= ) && > - (KeyData.KeyState.KeyShiftState !=3D EFI_SHIFT_STATE_VALID)) { > - // > - // Shift key pressed. > - // > + ModifiersPressed =3D ((KeyData.KeyState.KeyShiftState & > EFI_SHIFT_STATE_VALID) !=3D 0) > + && (KeyData.KeyState.KeyShiftState !=3D > + EFI_SHIFT_STATE_VALID); > + > + // > + // TRUE if Shift is pressed and no other modifiers are pressed > + // > + ShiftPressed =3D ModifiersPressed && > + ((KeyData.KeyState.KeyShiftState & > + ~(EFI_SHIFT_STATE_VALID | EFI_LEFT_SHIFT_PRESSED | > + EFI_RIGHT_SHIFT_PRESSED)) =3D=3D 0); > + > + if (ModifiersPressed && !ShiftPressed) { > continue; > } > // > // pressed ESC > // > - if (KeyData.Key.ScanCode =3D=3D SCAN_ESC) { > + if (!ModifiersPressed && KeyData.Key.ScanCode =3D=3D SCAN_ESC) { > Size =3D 0; > Status =3D EFI_NOT_READY; > break; > @@ -198,9 +206,10 @@ InputBarRefresh ( > // > // return pressed > // > - if (KeyData.Key.UnicodeChar =3D=3D CHAR_LINEFEED || > KeyData.Key.UnicodeChar =3D=3D CHAR_CARRIAGE_RETURN) { > + if (!ModifiersPressed > + && (KeyData.Key.UnicodeChar =3D=3D CHAR_LINEFEED || > + KeyData.Key.UnicodeChar =3D=3D CHAR_CARRIAGE_RETURN)) { > break; > - } else if (KeyData.Key.UnicodeChar =3D=3D CHAR_BACKSPACE) { > + } else if (!ModifiersPressed && KeyData.Key.UnicodeChar =3D=3D > + CHAR_BACKSPACE) { > // > // backspace > // > @@ -213,7 +222,8 @@ InputBarRefresh ( >=20 > } > } > - } else if (KeyData.Key.UnicodeChar <=3D 127 && KeyData.Key.UnicodeCh= ar >=3D > 32) { > + } else if ((!ModifiersPressed || ShiftPressed) > + && KeyData.Key.UnicodeChar <=3D 127 && > + KeyData.Key.UnicodeChar >=3D 32) { > // > // VALID ASCII char pressed > // > diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/EditMenuBar.c > b/ShellPkg/Library/UefiShellDebug1CommandsLib/EditMenuBar.c > index ca8bc506d9..eabbf3c571 100644 > --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/EditMenuBar.c > +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/EditMenuBar.c > @@ -190,11 +190,17 @@ MenuBarDispatchControlHotKey ( > // > // For consoles that supports/reports shift state, > // make sure only CONTROL is pressed. > + // For some consoles that report shift state, > + // Ctrl+A is still translated to 1 (UnicodeChar). > // > if ((KeyData->Key.UnicodeChar >=3D L'A') && (KeyData->Key.UnicodeCha= r <=3D > L'Z')) { > ControlIndex =3D KeyData->Key.UnicodeChar - L'A' + 1; > } else if ((KeyData->Key.UnicodeChar >=3D L'a') && (KeyData->Key.Uni= codeChar > <=3D L'z')) { > ControlIndex =3D KeyData->Key.UnicodeChar - L'a' + 1; > + } else if ((KeyData->Key.UnicodeChar >=3D L'A' - L'A' + 1) && (KeyDa= ta- > >Key.UnicodeChar <=3D L'Z' - L'A' + 1)) { > + ControlIndex =3D KeyData->Key.UnicodeChar; > + } else if ((KeyData->Key.UnicodeChar >=3D L'a' - L'a' + 1) && (KeyDa= ta- > >Key.UnicodeChar <=3D L'z' - L'z' + 1)) { > + ControlIndex =3D KeyData->Key.UnicodeChar; > } > } > if ((SCAN_CONTROL_Z < ControlIndex) > diff --git > a/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/MainHexEditor.c > b/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/MainHexEditor.c > index a00df49d38..394e531c16 100644 > --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/MainHexEditor.c > +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/MainHexEditor. > +++ c > @@ -137,14 +137,17 @@ HMainCommandDisplayHelp ( > if (KeyData.Key.UnicodeChar =3D=3D L'W' - L'A' + 1) { > break; > } > - } else if (((KeyData.KeyState.KeyShiftState & EFI_SHIFT_STATE_VALID)= !=3D 0) > && > - ((KeyData.KeyState.KeyShiftState & (EFI_LEFT_CONTROL_PRES= SED | > EFI_RIGHT_CONTROL_PRESSED)) !=3D 0) && > - ((KeyData.KeyState.KeyShiftState & ~(EFI_SHIFT_STATE_VALI= D | > EFI_LEFT_CONTROL_PRESSED | EFI_RIGHT_CONTROL_PRESSED)) =3D=3D 0)) { > + } else if (((KeyData.KeyState.KeyShiftState & EFI_SHIFT_STATE_VALID)= !=3D 0) > + && ((KeyData.KeyState.KeyShiftState & (EFI_LEFT_CONTROL_PRES= SED | > EFI_RIGHT_CONTROL_PRESSED)) !=3D 0) > + && ((KeyData.KeyState.KeyShiftState & > + ~(EFI_SHIFT_STATE_VALID | EFI_LEFT_CONTROL_PRESSED | > + EFI_RIGHT_CONTROL_PRESSED)) =3D=3D 0)) { > // > // For consoles that supports/reports shift state, > // make sure that only CONTROL shift key is pressed. > + // For some consoles that report shift state, > + // CTRL+W is still translated to L'W' - L'A' + 1. > // > - if ((KeyData.Key.UnicodeChar =3D=3D 'w') || (KeyData.Key.UnicodeCh= ar =3D=3D 'W')) > { > + if ((KeyData.Key.UnicodeChar =3D=3D 'w') || (KeyData.Key.UnicodeCh= ar =3D=3D 'W') > + || (KeyData.Key.UnicodeChar =3D=3D L'w' - L'a' + 1) || > + (KeyData.Key.UnicodeChar =3D=3D L'W' - L'A' + 1)) { > break; > } > } > -- > 2.21.1 (Apple Git-122.3) >=20 >=20 > -=3D-=3D-=3D-=3D-=3D-=3D > Groups.io Links: You receive all messages sent to this group. >=20 > View/Reply Online (#54122): https://edk2.groups.io/g/devel/message/54122 > Mute This Topic: https://groups.io/mt/71133729/1768756 > Group Owner: devel+owner@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub [zhichao.gao@intel.com= ] > -=3D-=3D-=3D-=3D-=3D-=3D