User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7) Gecko/20040517 Camino/0.8b Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7) Gecko/20040517 Camino/0.8b When using Type Ahead Find in 0.8b, pressing the spacebar causes the page to scroll down a few lines (similar to what happens when you press "page down"). Rather, a space should be inserted in the search. Reproducible: Always Steps to Reproduce: 1. press / to commence Type Ahead Find 2. type some letters then press spacebar 3. Actual Results: The page scrolls down (similar to what happensn when you press "page down"). Expected Results: A space should be inserted in the search.

Right on Brian, I remeber once seen this but never really tought about it. But it is indeed incorrect behaviour. Mozilla and FF behave the way we want Camino to do as well.

This doesn't sound too hard. I might take

i looked into this a tiny bit, the space gets to the TAF code (nsTypeAheadFind.cpp::KeyPressed) but it is ignored because something has set the preventDefault flag in the event record. any ideas why this would be the case? cc'ing aaronl

note that the page doesn't scroll until after the TAF has timed out (4 seconds). before then, the find is active just every space is ignored. just clarifying the steps to repro.

I think this has to do with the order of events. In Mozilla, Type Ahead Find apparently sees the spacebar before the code that would cause a page down sees it. It gets the first shot at the spacebar, so it can decide whether it's needed. For some reason Camino probably looks at the event before TAF does, and Camino doesn't know whether TAF is currently active. However, Camino could look at the isActive property in the nsITypeAheadFind service, and only pagedown/setpreventdefault if that's false.

aaron, you didn't read my comment fully. TAF gets the space but "preventDefault" is set on it so TAF returns early and nothing happens, at least on the trunk. I tried it on the branch and it appears that the space cancels TAF then causes a pagedown (gecko keybindings). We're seeing different behavior between trunk and branch. Sigh.

(In reply to comment #6) > aaron, you didn't read my comment fully. TAF gets the space but "preventDefault" > is set on it so TAF returns early and nothing happens, at least on the trunk. I read it, but maybe I'm just missing a few cards in my deck. If something else gets the event first, won't it set the preventDefault flag? Isn't that the only reason that flag would already be set when typeaheadfind gets the event?

but there's nothing in the camino event chain that would eat the space and set preventDefault. the only other thing that wants it would be the gecko keybindings, but those should all be handled in the same way as in FF. it's all a black box to camino.

What happens when you set a breakpoint here? http://lxr.mozilla.org/seamonkey/source/content/events/src/nsDOMEvent.cpp#372 Who's setting prevent default for the space?

This bug only occurs if you have autostart turned of. I you enable autostart howe ever using the space bar while doing a fnd as you tupe works perfectly. Add this line to your user.js file and the bug is gone! I wonder if this is perhaps a core bug? user_pref("accessibility.typeaheadfind.autostart", true);

Ping. Since this bug is targetted for 0.9 (Josh, should this be a 0.9 blocker?), has anyone looked at it recently? Flagging camin0.9?

no need to flag it, it's already targetted at 0.9

What's happening here is that the XBL keybindings that map the spacebar to Page Down are calling PreventDefault() on the keypress event before it gets to TAF. The page down command doesn't actually happen because TAF throws us into 'browse with caret' mode, and there is no cmd_scrollPageDown command registerd when in that mode. It seems like the behavior here is a crapshoot, depending on what order the XBL handers vs. TAF get registered. They are being called from the same loop in nsEventListenerManager::HandleEvent(); in camino, the XBL handlers are at index 1, and TAF is at index 2. What determines what order things are registered here?

I see that TAF registers its events listeners when you hit the / key, so they'll always come after the spacebar handler. So I don't see how this could ever work. In DeerPark, / brings up the TAF bar in the window. In that, typing a space seems to terminate the search, so I can't meaningfully compare. It may be broken in the same way.

(In reply to comment #14) > In DeerPark, / brings up the TAF bar in the window. In that, typing a space > seems to terminate the search, so I can't meaningfully compare. It may be broken > in the same way. Maybe I don't understand, but in DeerPark, on http://www.mozilla.org/projects/deerpark/ typing /deer park finds "Deer Park" in "what's new in Deer park" and hitting another space takes me down to "Deer Park " in "Deek Park Alpha 1". This is what Fx has done all along IIRC. My Deerk Park is from the 20th, so it could have broken since then, or I could just be misunderstanding comment 14; if so, apologies for the noise.

Here's what I did in my DP dev build: 1. type / and the TAF panel shows up at the bottom of the window 2. type some letters; they go into the text field, and are found (if present) 3. type a space; the TAF textfield goes red, it beeps (I think) and doesn't do another find.

I just installed the June 28 nightly DeerPark and can't repro comment 16, with user_pref("accessibility.typeaheadfind.autostart", true); set to either true or false (it defaults to true in DeerPark). I went back to Fx 0.9.1, the last release before the Find toolbar; with autostart true (the default), it suffers from the same page down (branch issue) as Camino in comment 1. With autostart off, Fx 0.9.1 finds spaces without issue. Since there are clear branch/trunk issues here, if someone could point me to the last trunk build of Fx without the Find toolbar, I'll happily jump through hoops with it, too :-) I used fresh profiles for each test and reloaded the Deer Park Alpha 1 page between about:config settings; I waited a few seconds after typing "deer" before typing the space. Are there other settings involved? Why are Simon and I getting different results?

is it going red for simon because the string he was looking for ("some chars" + a space) isn't actually in the document? So it was failing because of the space, but in the same way as any other set of chars that wouldn't be found?

I'd like this for 1.0. I also want [esc] to cancel the find. (and I'll do it myself if I have to)

(In reply to comment #19) > I'd like this for 1.0. In the interim, someone mentioned somewhere that opt-space works for entering space for some reason, as another work-around without turning on autostart. And as I mentioned before, if anyone can point me to when the Fx Find toolbar landed on the trunk, I'll be happy to do some end-user investigation into what happens in builds prior to that to see if it had the same bug as Camino has (so far I've been unable to find the bug/checkin info that would provide me that date). >I also want [esc] to cancel the find. If you find what's eating Esc, presumably that will fix or help fix bug 314306, too!

Per mento's comment, requesting blocking. :D Good luck Mark!

Might also want to look at bug 176037.

And another thing -- can we change the " -- " in the TAF strings to em dashes? It looks so fugly now.

i'd love to have this for 1.0, but it's not looking good.

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

Mark's been busy fixing Intel issues, so pushing out.

(In reply to comment #25) > *** Bug 321338 has been marked as a duplicate of this bug. *** > Is this correct? The behaviour is slightly different -- no downward scrolling in 321338: |Actual Results: |It will search "Mozillafoundation" with no space in it I have no idea whether this is significant or whether the bug has just been partially fixed.

(In reply to comment #27) > (In reply to comment #25) > > *** Bug 321338 has been marked as a duplicate of this bug. *** > > > > Is this correct? The behaviour is slightly different -- no downward scrolling > in 321338: > > |Actual Results: > |It will search "Mozillafoundation" with no space in it > > I have no idea whether this is significant or whether the bug has just been > partially fixed. > This is correct. comment 4 explains the behaviour and clarifies the steps to repro.

Comment 6 explains the different behavior; in Camino 0.8, space paged-down. In Camino 0.9 and above, the space is just eaten.

'K. Here's a few frames worth of what happens when preventDefault is set on the space key event: Breakpoint 1, nsDOMEvent::PreventDefault (this=0x25fac80) at /Users/mark/lizard/camino-10/mozilla/content/events/src/nsDOMEvent.cpp:382 382 if (!(mEvent->flags & NS_EVENT_FLAG_CANT_CANCEL)) { (gdb) bt #0 nsDOMEvent::PreventDefault (this=0x25fac80) at /Users/mark/lizard/camino-10/mozilla/content/events/src/nsDOMEvent.cpp:382 #1 0x07492364 in nsDOMKeyboardEvent::PreventDefault (this=0x25fac70) at /Users/mark/lizard/camino-10/mozilla/content/events/src/nsDOMKeyboardEvent.h:57 #2 0x07028c3c in nsXBLPrototypeHandler::ExecuteHandler (this=0x1e007030, aReceiver=0x25f31d0, aEvent=0x25fac80) at /Users/mark/lizard/camino-10/mozilla/content/xbl/src/nsXBLPrototypeHandler.cpp:352 #3 0x07023b10 in nsXBLWindowHandler::WalkHandlersInternal (this=0x1cb53374, aEvent=0x25fac80, aEventType=0x2829d58, aHandler=0x1e006dd0) at /Users/mark/lizard/camino-10/mozilla/content/xbl/src/nsXBLWindowHandler.cpp:305 #4 0x07024ddc in nsXBLWindowKeyHandler::WalkHandlers (this=0x1cb53370, aKeyEvent=0x25fac80, aEventType=0x2829d58) at /Users/mark/lizard/camino-10/mozilla/content/xbl/src/nsXBLWindowKeyHandler.cpp:193 #5 0x07024628 in nsXBLWindowKeyHandler::KeyPress (this=0x1cb53370, aKeyEvent=0x25fac80) at /Users/mark/lizard/camino-10/mozilla/content/xbl/src/nsXBLWindowKeyHandler.cpp:248 #6 0x06f23b08 in DispatchToInterface (aEvent=0x25fac80, aListener=0x1cb53370, aMethod=invalid pointer to member function ) at /Users/mark/lizard/camino-10/mozilla/content/events/src/nsEventListenerManager.cpp:141 #7 0x06f2beb0 in nsEventListenerManager::HandleEvent (this=0x25f3200, aPresContext=0x1cbe7ba0, aEvent=0xbfffe694, aDOMEvent=0xbfffdee8, aCurrentTarget=0x25f31d0, aFlags=514, aEventStatus=0xbfffe2ac) at /Users/mark/lizard/camino-10/mozilla/content/events/src/nsEventListenerManager.cpp:1778 PreventDefault is called from here: http://lxr.mozilla.org/mozilla1.8.0/source/content/xbl/src/nsXBLPrototypeHandler.cpp#350 Commenting that line out allows space to function in type-ahead find in Camino. I don't know if it breaks anything. I do know that long after that code appeared, another (conditional) call to PreventDefault was added earlier in the method: http://lxr.mozilla.org/mozilla1.8.0/source/content/xbl/src/nsXBLPrototypeHandler.cpp#216 I'm not an XBL guru, so I'm not really positive about how this stuff is supposed to work.

Aaron, is the above inspiring at all? We can't check nsITypeAheadFind's isActive property in content/xbl. I can't see how this would have worked in SeaMonkey (but my next project is to give that a try.) Ideas?

Escape is being eaten for another reason, and we'll need a hack here to deal with it: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/camino/src/browser/BrowserWindow.mm&rev=1.12.2.3.2.2#74 This patch implements such a hack, but it might not do the right thing if TAF is active for a window that's not the one you press escape in. I'm not sure if that's a possible state to get into, though. I briefly considered a similar hack to handle the space key (even though it's the wrong fix), but it would mean faking up a whole nsIDOMEvent. I also noticed that in Camino, we can't use TAF to search for a string that contains a forward slash or single quote. Those characters restart TAF. They're searchable in SeaMonkey.

Mark: see bug 314306 for escape key fixing (which we can do post-10.2).

Comment on attachment 211674 [details] [diff] [review] Patch to make Esc key cancel TAF Bug 314306 fixed the Esc key issue for 1.1 and above.

Mark, do you still have time to work on this? It's almost all the space bar issue now as Esc was fixed elsewhere.

OK, the behavior has changed *again*. Grr :/ With autostart off (which is the condition this bug has always been about), on the 1.8.1 branch and trunk, pressing space now immediately causes a "Find Stopped" message in the status bar and we immediately page down. (On 1.8.0 branch, and the 1.8.1 branch and trunk until "recently", we just ate the space entirely and went on to the next letter. On 1.7 branch, it seems that the page scrolled after FAYT timed out--comment 4.) Also, please, no comments unless you're someone working to solve this bug; the bug report is already in a very messy state that makes it hard to work. Workarounds are to enable autostart (comment 10) or to use opt-space instead to enter a space.

Mark, did you ever figure out how it is that this works in things other than Camino?

Minusing for 1.1.

Sorry for the spam, just wanted to link to CaminoSpace (<http://willmore.eu/plugins/caminospace.html>): * it *might* be interesting for developers to look at how it works * even if it simply wraps a known workaround, better remember to tell users/its developer it has become useless when this bug gets fixed! (BTW Camino 1.5 simply ignores the space but using opt-space instead still works.)

We are aware of CaminoSpace. What it does is a hack, not an actual fix. As for option-space, that was covered in comment 36. Please read the entire bug before commenting, as this bug is already noisy enough as it is.

Mass un-setting milestone per 1.6 roadmap. Filter on RemoveRedonkulousBuglist to remove bugspam. Developers: if you have a patch in hand for one of these bugs, you may pull the bug back to 1.6 *at that point*.

As far as I can tell, this works correctly on trunk (Camino 2.0a1) now. I'm not sure what solved this, perhaps bug 420699? On this page, I typed 'Filter on' and FAYT brought me to comment 41 above (with the string highlighted).

(In reply to comment #42) > As far as I can tell, this works correctly on trunk (Camino 2.0a1) now. I'm not > sure what solved this, perhaps bug 420699? > > On this page, I typed 'Filter on' and FAYT brought me to comment 41 above (with > the string highlighted). > Or not. It seems the browser was confused about the focussed state somehow (ala bug 424906). Sorry for the false hope.