298894 - [Mac] Clean up the enter key behavior mess

Status: RESOLVED FIXED

(Core :: XUL, defect, P2)

Description

[Asaf Romano (gone)]

Exculding cases like focused-textarea, the enter key in native mac dialogs
always fires the default button, even if another button is focused.

In dialog.xml, we have some sort of hack to make this also happen in our dialog
implementaion (search for enterDefaultAlwyas), however it doesn't work all that
well:

- (In some cases?) both buttons are fired (the focused one and the defualt one).
- When the default button is disabled, enter will fire the focused button (it
should never happen). [STR:
  1. Open the add bookmark dialog
  2. Remove the suggested title -> default button is disbaled.
  3. Focus the cancel button and hit enter. -> Dialog is closed.
- Wizards doesn't have the same hack, the UE is very different and unexpected.

Comment 1

[Asaf Romano (gone)]

Attachment: v1

This change removes the button keypress handler from the mac build and moves
the prevent default bit into nsButtonBoxFrame. The enterDefaultAlawys isn't
necessary anymore.

My suite tree is a bit old so I didn't yet have a chance to test it there.

Comment 2

[jhp (no longer active)]

Comment on attachment 187398 [details] [diff] [review]
v1

+	     aEvent->PreventDefault();

What's the point of this?

Comment 3

[Asaf Romano (gone)]

This is the "c++ replacement" to the orginal bit from the button binding
(button.xml). The "_hitEnter" method in the button binding checks for
preventDefault (to make sure it doesn't fire the default button when another
button is focused).

Comment 4

[jhp (no longer active)]

Comment on attachment 187398 [details] [diff] [review]
v1

Well, I can't comment on whether it is better to have the Mac specific code in
nsButtonBoxFrame.cpp or in XUL, but the actual code looks fine.

Comment 5

[Asaf Romano (gone)]

Checking in layout/xul/base/src/nsButtonBoxFrame.cpp;
/cvsroot/mozilla/layout/xul/base/src/nsButtonBoxFrame.cpp,v  <-- 
nsButtonBoxFrame.cpp
new revision: 1.28; previous revision: 1.27
done
Checking in toolkit/content/widgets/button.xml;
/cvsroot/mozilla/toolkit/content/widgets/button.xml,v  <--  button.xml
new revision: 1.10; previous revision: 1.9
done
Checking in toolkit/content/widgets/dialog.xml;
/cvsroot/mozilla/toolkit/content/widgets/dialog.xml,v  <--  dialog.xml
new revision: 1.22; previous revision: 1.21
done
Checking in xpfe/global/resources/content/bindings/button.xml;
/cvsroot/mozilla/xpfe/global/resources/content/bindings/button.xml,v  <-- 
button.xml
new revision: 1.25; previous revision: 1.24
done
Checking in xpfe/global/resources/content/bindings/dialog.xml;
/cvsroot/mozilla/xpfe/global/resources/content/bindings/dialog.xml,v  <-- 
dialog.xml
new revision: 1.31; previous revision: 1.30
done
Checking in xpfe/global/resources/content/mac/platformDialog.xml;
/cvsroot/mozilla/xpfe/global/resources/content/mac/platformDialog.xml,v  <-- 
platformDialog.xml
new revision: 1.8; previous revision: 1.7
done

Comment 6

[Asaf Romano (gone)]

backed out due to bustage...

Comment 7

[Asaf Romano (gone)]

Attachment: patch

Comment 8

[Asaf Romano (gone)]

Without the random downloads.js change in the last patch, checked in.