keyboard activation of <html:a> in an XUL page needs to prevent activation of the default "OK" button

RESOLVED FIXED in mozilla1.7alpha

Status

()

defect
RESOLVED FIXED
16 years ago
4 months ago

People

(Reporter: benjamin, Assigned: benjamin)

Tracking

(Blocks 1 bug, {access})

Trunk
mozilla1.7alpha
x86
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

This problem occurs in the seamonkey preferences->Theme panel with the link "Get
New Themes", but I need it for an installer project also.

<xul:dialog>
  <html:a onclick="dosomething(); return false;">Do something!</html:a>

  <xul:button default="true" label="Cancel" />
</xul:dialog>

If I mouse-click on the <a> link, everything is fine. If I tab to the link and
press enter, I activate the link *and* the default (OK) button. I am presuming
that the keyboard event handler for <html:a> should call preventDefault on the
keyboard event for "Enter" but I'm not quite sure where or how this occurs.

--BDS
See nsHTMLAchorElement::HandleDOMEvent
bryner, could you take a look at this little 8-line patch and tell me if I'm
doing something obviously wrong? My theory was to call preventDefault() on the
event and then check for getPreventDefault() in the dialog enter handler. Neil
was looking at this and musing about the dialog receiving the event before the
<a> handled it (I don't know enough about event ordering to make that make
sense).

--BDS
Posted patch woohoo (obsolete) — Splinter Review
Woohoo! Neil and I have come up with a patch that seems to work and gets rid of
the hackery in the dialog keypress handler:

Neil added the code that allows <xbl:handler phase="system">. Then we just had
to make sure that "enter" events were being preventdefaulted in the correct
cases. I made a test dialog and tested buttons, html input and textarea, xul
single and multi-line textbox, radio, checkbox, and of course my html:a. I also
navigated around the seamonkey preferences dialog without seeing any obvious
problems, and the "Get new themes" link now works correctly.
Attachment #136289 - Attachment is obsolete: true
Assignee: events → bsmedberg
OS: Windows 2000 → All
QA Contact: ian → neil.parkwaycc.co.uk
Target Milestone: --- → mozilla1.7alpha
Attachment #136377 - Flags: superreview?(jst)
Attachment #136377 - Flags: review?(bryner)
Comment on attachment 136377 [details] [diff] [review]
woohoo

Doh, just figured out why I wasn't able to do this before - I had written if
(event.preventDefault) return; :-[
Comment on attachment 136377 [details] [diff] [review]
woohoo

OK, so I've found an issue with this - the event.preventDefault() in
autocomplete.xml isn't reflected in the dialog.xml's event.
You mean this one?
http://lxr.mozilla.org/mozilla/source/xpfe/components/autocomplete/resources/content/autocomplete.xml#919
Are you sure that code is being triggered?

--BDS
Well, the preventBubble() works when it's a standard bubbling listener...
I've double-checked and the autocomplete.xml code is definitely triggered.
Comment on attachment 136377 [details] [diff] [review]
woohoo

OK, need to check the preventDefault attribute for dialog cancel also, to fix
Neil's autocomplete problem.
Attachment #136377 - Attachment is obsolete: true
Attachment #136377 - Flags: superreview?(jst)
Attachment #136377 - Flags: review?(bryner)
Attachment #136738 - Flags: superreview?(jst)
Attachment #136738 - Flags: review?(bryner)
This bug got me thinking about the Mac-specific code; it looks as if it should
install a capturing enter event listener, otherwise if you focus a button
(assuming that you changed the accessibility options to allow button focus) and
hit enter it would activate the button's command and also close the dialog.
Comment on attachment 136738 [details] [diff] [review]
make the cancel event do preventDefault checking as well

This isn't a correct implementation of the system event group, in general.  The
system event group is not a phase like capturing and bubbling -- there are both
capturing and bubbling phases within the system event group.  You will likely
need to choose an attribute other than phase= to use for this.
Attachment #136738 - Flags: review?(bryner) → review-
Attachment #136738 - Flags: superreview?(jst)
Blocks: 282380
Posted patch group="system" (obsolete) — Splinter Review
Attachment #136738 - Attachment is obsolete: true
Attachment #174682 - Flags: superreview?(bryner)
Attachment #174682 - Flags: review?(jst)
Whoops, forgot to update the actual XBL bindings :-[
Attachment #174682 - Attachment is obsolete: true
Attachment #174683 - Flags: superreview?(bryner)
Attachment #174683 - Flags: review?(jst)
Comment on attachment 174682 [details] [diff] [review]
group="system"

clearing obsolete request
Attachment #174682 - Flags: superreview?(bryner)
Comment on attachment 174683 [details] [diff] [review]
Really group="system"

Boris might be a good reviewer for this as well.
Attachment #174683 - Flags: superreview?(bryner) → superreview+
Comment on attachment 174683 [details] [diff] [review]
Really group="system"

r=jst
Attachment #174683 - Flags: review?(jst) → review+
Comment on attachment 174682 [details] [diff] [review]
group="system"

patch is obsolete, removing review request
Attachment #174682 - Flags: review?(jst)
Fix plus bustage fix checked in to xpfe.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Anyone feel like porting this to Firefox?

And doing it ASAP?  Looks like that's needed for bug 294262
nm. Being taken care of in bug 294290

Sorry for the bugspam
Blocks: 294290
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.