214373 - enter does not work on image maps

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Rick van der Sluijs]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5a) Gecko/20030728 Mozilla Firebird/0.6.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5a) Gecko/20030728 Mozilla Firebird/0.6.1

When you tab trough an image map, pressing enter or ctrl+enter when the focus
ring is around one of the areas doesn't do anything. I think it should open the
associated URL, because when you press shift+F10 and choose 'open link in new
tab' it will open the URL.

Reproducible: Always

Steps to Reproduce:
1.goto http://members.chello.nl/j.vandersluijs/moztest/
2.press tab a few times until the focus ring is around one of the buttons
3.press enter

Actual Results:  
nothing happens

Expected Results:  
Open the correct link in the same window

Comment 1

[Boris Zbarsky [:bzbarsky]]

Confirming; I thought this used to work...

Comment 2

[Aaron Leventhal]

Man, when did this break?

Comment 3

[amutch]

Not sure if this is related but clicking on the image here titled "download"
which is an image map doesn't work when clicked on with the mouse.

http://www.myob.com/us/downloads/downloads/mac_applescripts.htm

Comment 4

[Anne (:annevk)]

*** Bug 228275 has been marked as a duplicate of this bug. ***

Comment 5

[Anne (:annevk)]

Made the title more searchable. Is this happening because AREA isn't really a
link, like A and LINK?

Comment 6

[Boris Zbarsky [:bzbarsky]]

This is a regression from bug 124789.  The basic problem is that
nsPresShell::HandleEvent no longer sets mCurrentEventContent from the ESM.  It
just sets mCurrentEventFrame and then later gets the content from the frame.

Problem is, for <area> elements the event frame is the imageframe but the event
content needs to be the <area> content.  This works for click events because of
the hack in nsImageFrame::GetContentForEvent, but key events end up getting
delivered to the <img> node instead of the <area> node in this testcase....

If I just add a call to GetFocusedContent (setting mCurrentEventContent) at the
same place in nsPresShell::HandleEvent that calls GetFocusedFrame, key events on
<area> start working again....

Comment 7

[Boris Zbarsky [:bzbarsky]]

*** Bug 232995 has been marked as a duplicate of this bug. ***

Comment 8

[Brian Ryner (not reading)]

Attachment: patch

This does what boris described.  This seems like the right fix... the problem
just didn't affect tabbing since PostHandleEvent works off of the frame, not
the content.

(The other way we could do this is to make nsImageFrame::GetContentForEvent
return the focused area for key events.  That seems unnecessary though.)

Comment 9

[Brian Ryner (not reading)]

Attachment: better patch

don't mess up the refcounting...

Comment 10

[Brian Ryner (not reading)]

Comment on attachment 144157 [details] [diff] [review]
better patch

Here's a little background info that might be helpful for reviewing:

When focus is in an image map, the ESM keeps the focused frame (used for
Pre/PostHandleEvent) set to the image frame and sets the focused content to the
area element.  We need to make sure mCurrentEventContent is set to the ESM's
mCurrentFocus, or we will end up calling nsImageFrame::GetContentForEvent() and
it does not know to return the focused area for key events.  (For non-imagemap
elements, the focused frame will simply return mContent when GetContentForEvent
is called).

Comment 11

[Johnny Stenback (:jst)]

Comment on attachment 144157 [details] [diff] [review]
better patch

r+sr=jst

Comment 12

[Brian Ryner (not reading)]

Comment on attachment 144157 [details] [diff] [review]
better patch

requesting approval for 1.7 final

Comment 13

[Asa Dotzler [:asa]]

Comment on attachment 144157 [details] [diff] [review]
better patch

a=asa (on behalf of drivers) for checkin to 1.7

Comment 14

[Brian Ryner (not reading)]

checked in.

Comment 15

[Mike Kaply [:mkaply]]

We suspect this caused regression:

https://bugzilla.mozilla.org/show_bug.cgi?id=307172

Comment 16

[Aaron Leventhal]

(In reply to comment #15)
> We suspect this caused regression:
> 
> https://bugzilla.mozilla.org/show_bug.cgi?id=307172

I can confirm that. And it's an issue in 1.7.x, branch and trunk.

Comment 17

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1527381