User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7) Gecko/20040722 Firefox/0.9.1+ Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7) Gecko/20040722 Firefox/0.9.1+ the previous versions of Firefox, hitting the "space" bar shifts a page down, and shift-spacebar a page up. so in the latest nightly builds, hitting spacebar goes directly to the help window, a little inconvinient is is not ? Reproducible: Always Steps to Reproduce: 1. hitting spacebar 2. help window opens 3. Actual Results: help window opens Expected Results: shifting a page downwarts

Might have been caused by bug 251165.

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

Confirming based on dupe.

Happens for me too. Driving me nuts. Page down still works. Mac OS X, build Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7) Gecko/20040728 Firefox/0.9.1+

(In reply to comment #1) > Might have been caused by bug 251165. Looks likely; that bug was checked in on the 19th, and I had to go back to the July 18 build to get rid of it.

this also occurs when I use / or ' to start find as you type, as well as when I just start typing with the FAYT pref enabled. it makes find as you type rather difficult to use on the Mac.

We need to correct this regression quickly.

I notice the same behaviour if I use VK_HELP on Windows. Since I don't have a Mac, I couldn't test it there. So VK_HELP doesn't work at all. That's bug 124393, which I reopened. I'd like to use Cmd+? instead, which has been suggested in bug 88739 comment 76, but that doesn't work either. I just filed bug 253680 because of that. So the only option for now is to #ifndef MAC_OSX the keyset.

I meant #ifndef XP_MACOSX of course. This removes the keyboard shortcut on Mac. The menuitem still works of course. I tested this with #ifndef XP_WIN. I'm adding <!ENTITY openHelpMac.commandkey2 "?"> to be able to use that if bug 253680 is fixed before 1.0.

Comment on attachment 154756 [details] [diff] [review] #ifndef XP_MACOSX the keyset Mike, you r'ed bug 251165, so I'm asking you to review this one as well.

I don't understand the root cause of this bug. Why would VK_HELP be triggered by a spacebar press or "/" or "'"? Seems like there is a bug in FAYT or the help system. I *really* don't like #ifdef'ing code just because it doesn't work on a particular platform (and no one seems to know why). Someone please explain how the keyset is mapping all these keys to open help.

Can anybody using a Mac tell me if switching off Find As You Type (in Tools->Options->Advanced->Accessibility) helps? Brade, I'm not ifdefing the code altogether, I'm just excluding Mac, because it doesn't work there right now. The code was intended to use openHelpMac.commandkey (which is VK_HELP) on Mac, and openHelp.commandkey (which is F1) on other platforms. I don't want to rewrite that altogether, that's why I suggest to put a #ifndef XP_MACOSX around the keyset.

> Can anybody using a Mac tell me if switching off Find As You Type (in > Tools->Options->Advanced->Accessibility) helps? no, it doesn't. if I turn off that pref, but manually initiate FAYT by hitting either the / or ' keys, the Help window pops up.

ok, here's what I think is going on: * VK_HELP is missing from the list in mozilla/content/xbl/src/nsXBLPrototypeHandler.cpp (around line 700 or so) * GetMatchingKeyCode returns 0 but other code seems to be looking for -1 for "not found" * 0 seems to mean match any key event note to self: VK_HELP is also missing from nsGuiEvent.h but I don't think that relates to this bug I think there needs to be another bug for implementing VK_HELP on Windows/Linux/other OS. F1 should give help if it's always help. There shouldn't be a need to #ifdef any of these.

This makes VK_WIN not be triggered by any key on Windows, so I guess it fixes the issue on Mac as well. Brade, are you sure we don't need a modifier in nsMacEventHandler.cpp? I mean VK_HELP should be triggered by Cmd+?, not by ? alone, because VK_HELP will be used without modifiers, like the Help key on Mac, and F1 on other platforms in the future.

Comment on attachment 154814 [details] [diff] [review] as suggested by brade Caillon, this is the patch you wanted to look at over the weekend :)

Comment on attachment 154814 [details] [diff] [review] as suggested by brade argh, wrong patch.

The change in nsMacEventHandler.cpp was suggested by brade in bug 253680 comment 5.

Comment on attachment 154815 [details] [diff] [review] as suggested by brade, the real one This is the one.

Comment on attachment 154815 [details] [diff] [review] as suggested by brade, the real one First, I'd separate these patches. They are related but are independent. The first patch is what is blocking this bug. The other patch really belongs in a different bug, specific to VK_HELP key events. The nsMacEventHandler.cpp change needs to be a little smarter. You need a check for commandkey (e.g. ((eventModifiers & cmdKey) != 0) before assigning it to VK_HELP (otherwise Mac users wouldn't be able to type a '?' character in an edit field)). A few more thoughts: modifier keys (shift) are probably getting set; do they need to be cleared for VK_HELP? We shouldn't set VK_HELP if control-shift-/ is pressed. The Mac code may already take that into account. Is someone going to fix the windows code (and linux code) to send VK_HELP events? r=brade on the nsXBLPrototypeHandler.cpp changes (although I'm not a module owner there)

OK, here are the changes to nsXBLPrototypeHandler.cpp.

Comment on attachment 155019 [details] [diff] [review] nsXBLPrototypeHandler.cpp only Caillon, this is the same as before, only excludes the nsMacEventHandler.cpp part.

I moved the Mac part of the patch to bug 253680.

Bug still present in Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7) Gecko/20040807 Firefox/0.9.1+ Space bar triggers help window.

Is someone going to review this? I'd like it to land before I test the Mac-specific patch.

Steffen, please send email to solicit review in addition to using the flags.

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

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

Comment on attachment 154756 [details] [diff] [review] #ifndef XP_MACOSX the keyset Requesting review on the ifndef-patch again.

Comment on attachment 154756 [details] [diff] [review] #ifndef XP_MACOSX the keyset r=me on the bandaid

Comment on attachment 154756 [details] [diff] [review] #ifndef XP_MACOSX the keyset Bandaaid until VK_HELP works.

Comment on attachment 155019 [details] [diff] [review] nsXBLPrototypeHandler.cpp only The patch doesn't work for me. I guess we should back to #ifndef XP_MACOSX, at least for the PR.

Asaf Romano (comment 32) -- could you be more specific about what doesn't work with the patch? Are you still seeing the space keypress opening the help?

(In reply to comment #33) > Asaf Romano (comment 32) -- could you be more specific about what doesn't work > with the patch? Are you still seeing the space keypress opening the help? Same problem as before (space still opens the help viewer), if you want i can check it one more time, but...

Brade, caillon said on IRC that he wants a better explanation for the "return -1" change, since that affects this code: 578 nsXBLPrototypeHandler::KeyEventMatched(nsIDOMKeyEvent* aKeyEvent) 579 { 580 if (mDetail == -1 && mMisc == 0 && mKeyMask == 0) 581 return PR_TRUE; // No filters set up. It's generic. 604 nsXBLPrototypeHandler::MouseEventMatched(nsIDOMMouseEvent* aMouseEvent) 605 { 606 if (mDetail == -1 && mMisc == 0 && mKeyMask == 0) 607 return PR_TRUE; // No filters set up. It's generic. <caillon> (I think the case for mouse event is stronger than key event in your case) <caillon> but still <caillon> should "unknown" equal "uninitialized"? if so, why? <caillon> I couldn't find a good reason to make that so in reading the source, but I assume you have one for making the patch. <caillon> I'd love to hear it <caillon> because it could very well be the case. <Steffen> caillon, brade said that GetMatchingKeyCode returns 0 but other code seems to be looking for -1 for "not found" <caillon> Steffen, right. other code does check for -1. which is the uninitialized state. <caillon> now, you're adding another path to get to that state, and I'd like more than "someone told me to do this" before you do it. :-) I'm afraid I couldn't explain it better.

Comment on attachment 155019 [details] [diff] [review] nsXBLPrototypeHandler.cpp only Brade and Mano said this doesn't fix the problem on Mac.

I rebuilt my seamonkey tree with extensions/help and the patches from Steffen in this bug as well as the xul keybinding patch for VK_HELP (thanks!) and in debug mode (make -f client.mk build). I don't see the problem. I did see the problem in my optimized build where I had only built extensions/help, content and xpfe/bootstrap. I contend the nsXBLPrototypeHandler.cpp only patch above is correct (at least the first block). More may be needed. I haven't read caillon's comments (above) yet. When I did see the bug, I thought it was interesting that typing a letter would also cause the scroll (not just space)--any key combination not already mapped to a keybinding. That may have varied based on what the focus was.

I believe this is the patch that should land to fix this bug. (It's the first half of the previous patch.) It fixes the bug for me. I am opposed to landing the #ifndef patch in this bug.

Comment on attachment 155965 [details] [diff] [review] minimal fix for this bug r=caillon

Comment on attachment 155965 [details] [diff] [review] minimal fix for this bug I guess I need sr as well. Brian, this is just a one-liner, and it fixes a nasty regression on Mac.

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

Comment on attachment 155965 [details] [diff] [review] minimal fix for this bug One line, regression fix, no risk.

Comment on attachment 155965 [details] [diff] [review] minimal fix for this bug Can someone check this in once it has approval, please? My cvs account is limited to browser/ and toolkit/.

patch checked into trunk: /cvsroot/mozilla/content/xbl/src/nsXBLPrototypeHandler.cpp,v <-- nsXBLPrototypeHandler.cpp new revision: 1.82; previous revision: 1.81 leaving bug open for aviary branch

Thanks, Brade! Can anybody confirm that this is fixed with Mac Firefox trunk (1.8a3) nightlies starting 08/14? Help contents shouldn't open on pressing any key but the Help key (the one left of the Home key).

Seems to be fixed here. Thanks!

> Can anybody confirm that this is fixed with Mac Firefox trunk (1.8a3) nightlies > starting 08/14? Looks good here with Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a3) Gecko/20040814 Firefox/0.9.1+ on osx 10.3.5

(In reply to comment #45) > Thanks, Brade! > > Can anybody confirm that this is fixed with Mac Firefox trunk (1.8a3) nightlies > starting 08/14? Help contents shouldn't open on pressing any key but the Help > key (the one left of the Home key). Same here with build 20040815 on Mac OS X 10.2.8 (first generation iBook) Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a3) Gecko/20040815 Firefox/0.9.1+ I can't test Cmd-Help though, the Help key is missing on my keyboard.

(In reply to comment #48) > (In reply to comment #45) > > Thanks, Brade! > > > > Can anybody confirm that this is fixed with Mac Firefox trunk (1.8a3) nightlies > > starting 08/14? Help contents shouldn't open on pressing any key but the Help > > key (the one left of the Home key). > > Same here with build 20040815 on Mac OS X 10.2.8 (first generation iBook) > > Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a3) Gecko/20040815 > Firefox/0.9.1+ > > I can't test Cmd-Help though, the Help key is missing on my keyboard. NO, the bug remains on Mac X 10.3.5... pressing the spacebar invokes the help page to show up, instead of a pagedown.

(In reply to comment #49) > NO, the bug remains on Mac X 10.3.5... > pressing the spacebar invokes the help page to show up, instead of a pagedown. are you sure you did your test with a latest **trunk** build (not latest-0.9)?

the bug is still there: pressing the SPACE bar invokes opening a new window with the help functions... this is VERY annoying !!! 240816

(In reply to comment #51) read comment 50, thanks.

I tested with the LATEST trunc (nighly build) rv. 1.7.2 Gecko/20040816 Firefox/0.9.1+

1.7.2 is probably from latest-0.9... not trunk

See comment 45. Trunk is 1.8a3.

ok, this seemes an other build than mine... although its from the SAME date. can you help me please, and say where I can find the build number of te browser Macintosch version, when I do CMD-I, I get always the wrong and same info... AND, the bug seemes FIXED !!! Thanks.

I am using the latest trunk build: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a3) Gecko/20040816 Firefox/0.9.1+ and it works perfectly for me. Both spacebar and any lettered key on the keyboard do not trigger the error anymore. Thanks for your good work. :)

Comment on attachment 155965 [details] [diff] [review] minimal fix for this bug a=asa for aviary checkin.

Can someone check this into the aviary branch, please?

Checked in on aviary. Leaving open for 1.7 branch to avoid forking Gecko between aviary and 1.7.x. Asa, can you approve for the 1.7 branch?

Thanks, Jesse! This is fixed finally.

I guess I should read the last comment first. Reopening.

-> Browser, so that I can request approval1.7.x.

Comment on attachment 155965 [details] [diff] [review] minimal fix for this bug One-line fix, no risk, tested on trunk and aviary branch.

Can somebody check this in for me on the MOZILLA_1_7_BRANCH, please?

fixed on 1.7 branch per comment 65 resolving as fixed