622246 - Drop down menu href links dont work when clicked

Status: RESOLVED FIXED

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

Description

[Yansky]

User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b9pre) Gecko/20101230 Firefox/4.0b9pre
Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b9pre) Gecko/20101230 Firefox/4.0b9pre

When you click on the "Sign In To..." on the right, a drop down menu appears. Clicking on any of the links should take you to a new page, but the browser does nothing after the onlick is done.

Reproducible: Always

Steps to Reproduce:
1. Go to http://www.westpac.com.au/
2. Click on the "Sign In To..." drop down menu.
3. Click on any of the links presented in the drop down menu.
Actual Results:  
The browser does not go to the new url page.

Expected Results:  
The browser should go to the new url page after clicking on a menu drop down link.

I don't think it is related to this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=622242 as I have disabled adblock plus and I am still encountering the error.

Also, I can't reproduce https://bugzilla.mozilla.org/show_bug.cgi?id=622242

Comment 1

[Alice0775 White]

Regression window:
Works
http://hg.mozilla.org/mozilla-central/rev/699023bb3c9c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9pre) Gecko/20101222
Firefox/4.0b9pre ID:20101227122945
Fails:
http://hg.mozilla.org/mozilla-central/rev/b7acaa08a9c5
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9pre) Gecko/20101223
Firefox/4.0b9pre ID:20101227124535
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=699023bb3c9c&tochange=b7acaa08a9c5

Comment 2

[Alice0775 White]

Attachment: semi-reduced html

Comment 3

[Alice0775 White]

Attachment: reduced html

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Actually reduced

Comment 5

[Boris Zbarsky [:bzbarsky]]

So what happens is that the targetContent returned by GetEventTargetContent on the ESM is the textnode inside the <a> (because that's the ESM's mCurrentTargetContent), not the <a> itself.

If I don't remove things from the DOM, then the ESM's mCurrentTargetContent is null, and we ask the presshell, which sees that its mCurrentEventContent is the <a> and returns that.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Ah, and that's because ESM sets mCurrentTargetContent from ClearFrameRefs... and then doesn't do anything interesting if there's event dispatch nested inside the handling of the other event.  smaug, is there a reason to actually use GetEventTargetContent here?  I stole that code from CheckHandleEventForAnchorsPreconditions, but perhaps I really don't want that here.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed fix

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 500549 [details] [diff] [review]
Proposed fix

 
>   case NS_UI_ACTIVATE:
>     {
>-      nsCOMPtr<nsIContent> targetContent;
>-      aVisitor.mPresContext->EventStateManager()->
>-        GetEventTargetContent(aVisitor.mEvent, getter_AddRefs(targetContent));
>-      if (targetContent == this) {
>+      if (aVisitor.mEvent->target == this) {

Could you use ->originalTarget here?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Yeah, I think I could.  I'll do that.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/95e3bf77d7d0 with that change

Comment 11

[LegNeato]

As per today's meeting, beta 9 will be a time-based release. Marking these all betaN+. Please move it back to beta9+ if  you believe it MUST be in the next beta (ie: trunk is in an unshippable state without this)