294882 - Onfocus for selects is triggered after dismissing the menu instead of before

Status: RESOLVED FIXED

(Camino Graveyard :: HTML Form Controls, defect)

Description

[Jamie Kahn Genet]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050518 Camino/0.8+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050518 Camino/0.8+

Go to any part of MacOrchard.com, e.g. FTP, Email or Usenet. Then select an
application from the pull down menu at the top of the main frame. Camino moves
the page down to the application's section, but then moves back up to the top of
the frame.

Reproducible: Always

Steps to Reproduce:
1.Go to <http://www.macorchard.com/>
2.Click on <http://www.macorchard.com/ftp.html>
3.Pull down the menu labeled 'Select a program to view:'
4.Select an application's name, e.g. Fetch
Actual Results:  
The page moves down to the selected application's section. Then it imediately
moves right back up to the top.

Expected Results:  
Moved down to the application's section and stayed there.

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Confirming.  I can also occasionally get a situation where selecting an item
does not move the page at all, but 99% of the time it jumps to the anchor and
the back to the top of the frame.

I can reproduce this without the frameset and table and so forth, with just the
<form>, <select>, and <script>.  Testcase coming.

Speculation: Some event seems to be misfiring or re-firing, but I don't think
it's any of the cases covered by existing Camino bugs: bug 258445, bug 168725. 
Or it's an anchor bug.  But I don't know enough about either.

Comment 2

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Simplified testcase

Comment 3

[Wevah (Nate Weaver)]

It works if you use the keyboard to select an entry (without actually opening
the popup) and pressing enter. It looks like a weird focus thing to me.

Comment 4

[Wevah (Nate Weaver)]

Aye, it seems like we're taking focus away from the select, and then reapplying
it after the menu item has been selected, which makes the page jump back to the
menu.

Comment 5

[Mike Pinkerton (not reading bugmail)]

this only happens in camino?

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

It works correctly in a Fx trunk build from earlier this month and in Fx 1.0.4;
it's broken in Camino trunk and 0.8.4...

Comment 7

[Mike Pinkerton (not reading bugmail)]

well that makes no sense ;)

Comment 8

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

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

Comment 9

[Simon Fraser [no longer active]]

This is probably related to Camino's hacky select implementation. We navigate to
the anchor on the mouse down, then on the way out, the code that focusses the
popup causes it to scroll to make that visible.

Comment 10

[Wevah (Nate Weaver)]

Yeah; optimally, we should be focusing the <select> before the menu is popped
up, not after it's dismissed.

Comment 11

[Wevah (Nate Weaver)]

I don't see this on trunk anymore, though admittedly I'm using a build that's a few weeks old (to avoid a layout bug on a site I use regularly).

Comment 12

[Wevah (Nate Weaver)]

To clarify: I don't see it on the testcase anymore.

Comment 14

[Stuart Morgan]

Attachment: fix

This causes us to show the menu asynchronously, so that the full Gecko handling of the mousedown/keydown will be processed first, making the focus happen at the right time relative to menu actions. It's a bit dodgy in that it's theoretically possible for a focus event to destroy the select out from under us, but this retains the select, so it's not dangerous (and none of this is any more dodgy that what we already do with the menu actions).

Comment 15

[Stuart Morgan]

The diff is a bit odd, but all I did is move the implementation of ShowNativeMenuForSelect (unmodified except for returns and some ObjC message style cleanup) into a new class that handles doing a 0-delay callback.

Comment 16

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Stuart, is this really going to be 2.0/trunk-only, or are we hoping to get this for 1.6 (and possibly 1.5.6 if it happens) for web-compat goodness?  (The patch doesn't apply on the 1_8 branch, but I haven't done any checking to see why.)

Comment 17

[Stuart Morgan]

This will be 1.6able as well, and I was planning on spinning a version for that when I landed it; the 2.0 milestone was from back when I was rounding up all the select bugs in the hope that we might be able to use Gecko for the menus in 2.0 (which isn't going to happen, unless the meuns are unexpectedly rewritten for Gecko 1.9)

Comment 18

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 301139 [details] [diff] [review]
fix

+  NSLog(@"Cleaning up popup displayer");

remove the log

sr=pink

Comment 19

[Stuart Morgan]

Attachment: v2

This is the fix as I will land it once the tree is open again; it removes the NSLog, and fixes a crash I missed in the case where the select is destroyed by onfocus.

Branch is exactly the same except for two context changes (nothing different in the changed code itself), so I'll just massage that on checkin.

Comment 20

[Stuart Morgan]

Landed on trunk and MOZILLA_1_8_BRANCH.

Comment 22

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Camino 1.5.6 is not happening.