[Mac] Clean up the enter key behavior mess

RESOLVED FIXED in mozilla1.8beta3

Status

()

Core
XUL
P2
normal
RESOLVED FIXED
13 years ago
9 years ago

People

(Reporter: mano, Assigned: mano)

Tracking

Trunk
mozilla1.8beta3
PowerPC
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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.
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla1.8beta3
Created attachment 187398 [details] [diff] [review]
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.
Attachment #187398 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #187398 - Flags: review?(jhpedemonte)
Attachment #187398 - Flags: review?(mconnor)
Comment on attachment 187398 [details] [diff] [review]
v1

+	     aEvent->PreventDefault();

What's the point of this?
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 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.
Attachment #187398 - Flags: review?(jhpedemonte) → review+

Updated

13 years ago
Attachment #187398 - Flags: review?(mconnor) → review+

Updated

13 years ago
Attachment #187398 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
Attachment #187398 - Flags: approval1.8b3?

Updated

13 years ago
Attachment #187398 - Flags: approval1.8b3? → approval1.8b3+
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
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
backed out due to bustage...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Created attachment 187624 [details] [diff] [review]
patch
Attachment #187398 - Attachment is obsolete: true
Attachment #187624 - Flags: superreview?(neil.parkwaycc.co.uk)

Updated

13 years ago
Attachment #187624 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
Without the random downloads.js change in the last patch, checked in.
Status: REOPENED → RESOLVED
Last Resolved: 13 years ago13 years ago
Resolution: --- → FIXED

Updated

13 years ago
Blocks: 300227

Updated

9 years ago
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.