41305 - [key] <ctrl>d incorrectly sets keycode

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect, P2)

Description

[rods (gone)]

On the Mac when a user presses <ctrl>d the method 
nsMacEventHandler::ConvertKeyEventToUnicode incorrectly clears the charCode and 
sets the keyCode to "4". Just before this call it correctly set the charCode to 
100 and had the keyCode cleared.

Comment 1

[Kathleen :Brade]

Accepting; Rod and I tracked this bug down to where the Unicode conversion is 
happening.  It's based on the event rather than the already converted character 
so we don't generate a 'd' or 'D' as appropriate.  :-(
This bug would be good to get fixed before we RTM.

Comment 2

[Kathleen :Brade]

I'd like to get this in for nsbeta2 but there are probably more important things 
to fix; adding nsbeta3 keyword since we need to fix this for RTM.

Comment 3

[rubydoo123]

ctrl keys on the mac are not set correctly, the expectation is that all ctrl 
keys work appropriately

Comment 4

[rubydoo123]

setting to nsbeta3+

Comment 5

[rubydoo123]

adding priority to status whiteboard, not necessarily difficult to fix, but will 
take time to fix it right.

Comment 6

[Kathleen :Brade]

reassign this bug to Frank
Frank--I'm leaving on sabbatical for 6 weeks and haven't gotten to fixing this 
bug yet (sorry!); I'm giving this particular bug to you since I think the problem 
is in the Unicode converter Mac toolbox call (or rather when we use that call).  
Akkana has offered to help review this.

to whom it may concern:  The implication of this bug is that key events on Mac 
with a control key modifier won't work in xul.  We should fix this before we ship 
so that any keybinding with a control modifier will work.

Comment 7

[Kathleen :Brade]

really give this bug to ftang; Frank please read above comments

Comment 8

[Frank Tang]

mark it as assign.
What does [p:2] mean ?

Comment 9

[rubydoo123]

hi Frank -- the [p:n] markings are our way of determining which to tackle first 
-- [p:1]-- high risk, hard to do, time consuming; [p:2]not as high a risk, 
may be hard to do, may take a lot of investigation, etc.

Comment 10

[Frank Tang]

Is there some test cases that I can use to verify this bug ?

Comment 11

[Akkana Peck]

You could add an XBL key binding for control-d to
xpfe/global/resources/skin/mac/platformHTMLBindings.xml, or a XUL binding to
communicator/resources/content/mac/platformBrowserBindings.xul, and then see if
the binding worked.

Comment 12

[Frank Tang]

akkana- rchen/tao- 
>You could add an XBL key binding for control-d to
>xpfe/global/resources/skin/mac/platformHTMLBindings.xml, or a XUL binding to
>communicator/resources/content/mac/platformBrowserBindings.xul, and then see if
>the binding worked.

thank you for your comment. But I really have no clue how to do that. Is that 
possible someone can provide a xul for me to do that ? 
beppe- why this bug is important ? IS there are test suite I can test against to 
verify it is working or not working? Can your QA create test cases for me ?

Comment 13

[Frank Tang]

add sfraser and jfrancis to the cc list.
sfraser Ctrl on Mac are used similar to the Opt key. They are used to type in 
special characters instead of as action key. We probably should do more research 
before fixing it.

Comment 14

[Frank Tang]

beppe-
Is that possible your engineer and/or QA can create a testing method for me? How 
can I know this bug is fix or not ? I really have no clue about XBL. It will be 
nice if your folks can give me something that I can verify the result.

Comment 15

[Akkana Peck]

For example, in xpfe/global/resources/skin/mac/platformHTMLBindings.xml, create
a binding under "inputFields" that says:

    <handler type="keypress" id="key_delforw" key="d" control="true"
        command="cmd_deleteCharForward"/>

(I just copied that line from the corresponding file in ../unix), then, in a
text field, see if control-d deletes the character the caret is on, the way it
does on Unix.

Comment 16

[Frank Tang]

I test by apply the following patch (not intend to check in, just for testing on
my local tree)
Index: platformHTMLBindings.xml
===================================================================
RCS file:
/cvsroot/mozilla/xpfe/global/resources/content/mac/platformHTMLBindings.xml,v
retrieving revision 1.1
diff -c -2 -r1.1 platformHTMLBindings.xml
*** platformHTMLBindings.xml	2000/05/29 02:54:01	1.1
--- platformHTMLBindings.xml	2000/08/23 00:47:33
***************
*** 7,11 ****
    <binding id="inputFields"
extends="chrome://global/content/htmlBindings.xml#inputFieldsBase">
      <handlers>
!
        <handler type="keypress" id="key_redo" key="z" primary="true"
shift="true" alt="false" control="false"
          command="cmd_redo"/>
--- 7,28 ----
    <binding id="inputFields"
extends="chrome://global/content/htmlBindings.xml#inputFieldsBase">
      <handlers>
!      <handler type="keypress" id="key_home" key="a" control="true"
!         command="cmd_beginLine"/>
!     <handler type="keypress" id="key_end" key="e" control="true"
!         command="cmd_endLine"/>
!     <handler type="keypress" id="key_left" key="b" control="true"
!         command="cmd_charPrevious"/>
!     <handler type="keypress" id="key_right" key="f" control="true"
!         command="cmd_charNext"/>
!     <handler type="keypress" id="key_delback" key="h" control="true"
!         command="cmd_deleteCharBackward"/>
!     <handler type="keypress" id="key_delforw" key="d" control="true"
!         command="cmd_deleteCharForward"/>
!     <handler type="keypress" id="key_delwback" key="w" control="true"
!         command="cmd_deleteWordBackward"/>
!     <handler type="keypress" id="key_del_bol" key="u" control="true"
!         command="cmd_deleteToBeginningOfLine"/>
!     <handler type="keypress" id="key_del_eol" key="k" control="true"
!         command="cmd_deleteToEndOfLine"/>
        <handler type="keypress" id="key_redo" key="z" primary="true"
shift="true" alt="false" control="false"
          command="cmd_redo"/>

Comment 17

[Frank Tang]

Here is my fix
Index: nsMacEventHandler.cpp
===================================================================
RCS file: /cvsroot/mozilla/widget/src/mac/nsMacEventHandler.cpp,v
retrieving revision 1.100
diff -c -6 -r1.100 nsMacEventHandler.cpp
*** nsMacEventHandler.cpp	2000/07/20 00:13:00	1.100
--- nsMacEventHandler.cpp	2000/08/23 00:50:14
***************
*** 785,824 ****
  	aKeyEvent.isMeta
	= ((aOSEvent.modifiers & cmdKey) != 0);
  	
  	//
  	// nsKeyEvent parts
  	//
  	if (message == NS_KEY_PRESS
! 	&& !IsSpecialRaptorKey((aOSEvent.message & keyCodeMask) >> 8) )
  	{
  	  if ( aKeyEvent.isControl )
  	  {
  	    aKeyEvent.charCode = (aOSEvent.message & charCodeMask);
  	    if ( aKeyEvent.charCode <= 26 )
  	    {
  	      if ( aKeyEvent.isShift )
  	        aKeyEvent.charCode += 'A' - 1;
  	      else
  	        aKeyEvent.charCode += 'a' - 1;
! 	    }
! 	  }
! 	  else
!  	  if ( !aKeyEvent.isMeta)
!     {
!       aKeyEvent.isShift = aKeyEvent.isControl = aKeyEvent.isAlt =
aKeyEvent.isMeta = 0;
!     }

!     aKeyEvent.keyCode	= 0;
!     aKeyEvent.charCode = ConvertKeyEventToUnicode(aOSEvent);
! 	  NS_ASSERTION(0 != aKeyEvent.charCode, "nsMacEventHandler::InitializeKeyEvent:
ConvertKeyEventToUnicode returned 0.");
! 	}
  	else
  	{
!     aKeyEvent.keyCode = ConvertMacToRaptorKeyCode(aOSEvent.message,
aOSEvent.modifiers);
!     aKeyEvent.charCode = 0;
!   }

    //
    // obscure cursor if appropriate
    //
    if ( message == NS_KEY_PRESS
    && !aKeyEvent.isMeta
--- 785,828 ----
  	aKeyEvent.isMeta
	= ((aOSEvent.modifiers & cmdKey) != 0);
  	
  	//
  	// nsKeyEvent parts
  	//
  	if (message == NS_KEY_PRESS
! 	    && !IsSpecialRaptorKey((aOSEvent.message & keyCodeMask) >> 8) )
  	{
  	  if ( aKeyEvent.isControl )
  	  {
  	    aKeyEvent.charCode = (aOSEvent.message & charCodeMask);
  	    if ( aKeyEvent.charCode <= 26 )
  	    {
  	      if ( aKeyEvent.isShift )
  	        aKeyEvent.charCode += 'A' - 1;
  	      else
  	        aKeyEvent.charCode += 'a' - 1;
! 	    } // if ( aKeyEvent.charCode <= 26 )

!         aKeyEvent.keyCode	= 0;
! 	  } // if ( aKeyEvent.isControl )
! 	  else // else for if ( aKeyEvent.isControl )
! 	  {
!  	  	if ( !aKeyEvent.isMeta)
!    		{
!      	  aKeyEvent.isShift = aKeyEvent.isControl = aKeyEvent.isAlt =
aKeyEvent.isMeta = 0;
!   		} // if ( !aKeyEvent.isMeta)
!
!         aKeyEvent.keyCode	= 0;
!         aKeyEvent.charCode = ConvertKeyEventToUnicode(aOSEvent);
! 	    NS_ASSERTION(0 != aKeyEvent.charCode,
"nsMacEventHandler::InitializeKeyEvent: ConvertKeyEventToUnicode returned 0.");
!       } // else for if ( aKeyEvent.isControl )
! 	} // if (message == NS_KEY_PRESS && !IsSpecialRaptorKey((aOSEvent.message &
keyCodeMask) >> 8) )
  	else
  	{
!       aKeyEvent.keyCode = ConvertMacToRaptorKeyCode(aOSEvent.message,
aOSEvent.modifiers);
!       aKeyEvent.charCode = 0;
!     } // else for  if (message == NS_KEY_PRESS &&
!IsSpecialRaptorKey((aOSEvent.message & keyCodeMask) >> 8) )

    //
    // obscure cursor if appropriate
    //
    if ( message == NS_KEY_PRESS
    && !aKeyEvent.isMeta


Basically, I just make it better indent.

sfraser, can you review my code. I will drop by your cube tomorrow for this.

Comment 18

[Frank Tang]

I place a new version of nsMacEventHandler.cpp  under http://warp/u/ftang/tmp/
nsMacEventHandler.cpp if you want to grab the new file. It is modified from r=
1.100

Also my modified version of 
/xpfe/global/resources/content/mac/platformHTMLBindings.xml  
(for testijng only) is under ftp://ftang@ftang/u/ftang/tmp/
platformHTMLBindings.xml

Comment 19

[Frank Tang]

sfraser- jfrancis review my widget changes. 
I will reassign the bug to you after I check in the widget changes.
you can decide how to add those key binding for Mac. Maybe we can use some of 
the Linux binding ??? It will be up to you. 

Comment 20

[Frank Tang]

Mark this bug as fix. Open new bug 50078 for the remaining control keybinding on 
Mac and assign to sfraser.

Comment 21

[sujay]

verified in 8/25 build.