Closed Bug 298894 Opened 17 years ago Closed 17 years ago

[Mac] Clean up the enter key behavior mess

Categories

(Core :: XUL, defect, P2)

PowerPC
macOS
defect

Tracking

()

RESOLVED FIXED
mozilla1.8beta3

People

(Reporter: asaf, Assigned: asaf)

References

Details

Attachments

(1 file, 1 obsolete file)

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
Attached patch v1 (obsolete) — Splinter Review
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+
Attachment #187398 - Flags: review?(mconnor) → review+
Attachment #187398 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
Attachment #187398 - Flags: approval1.8b3?
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
Closed: 17 years ago
Resolution: --- → FIXED
backed out due to bustage...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch patchSplinter Review
Attachment #187398 - Attachment is obsolete: true
Attachment #187624 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #187624 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
Without the random downloads.js change in the last patch, checked in.
Status: REOPENED → RESOLVED
Closed: 17 years ago17 years ago
Resolution: --- → FIXED
Blocks: 300227
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.