User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; nl; rv:1.8.1.9) Gecko/20071025 Firefox/2.0.0.9 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; nl; rv:1.8.1.9) Gecko/20071025 Firefox/2.0.0.9 Can't use symbol in text fields like the euro symbol: € (Ctrl+Alt+5) becouse when you hit Ctrl+5, you get the 5th tab. Reproducible: Always Steps to Reproduce: 1. Go to a text-field 2. Make a Euro symbol or other symbol you make with Ctrl+Alt+[number] 3. Firefox sees it like Ctrl+[number], so you get a tab Actual Results: You get an other tab and not the symbol Expected Results: Make the symbol and not show a tab.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007111023 Minefield/3.0b2pre The alt gr key ( http://nl.wikipedia.org/wiki/Alt_Gr ), present on my laptop keyboard, works well here to make euro's: €€€. Also Ctrl+Alt + 5 makes euro's here. €€€. I go indeed to the fifth tab if I first press Ctrl+5 and then Alt.

This patch makes the tab-switching shortcuts more precise. For example, on Mac, it makes them only trigger on Cmd+number and not on Cmd+Ctrl+number or Cmd+Alt+number. I'll ask someone to test on Windows trunk (both without and with the patch) before requesting review from Mano.

I'd like to confirm the bug: on Windows XP using Ctrl+Alt+number or Ctrl+Shift+number selects the numbered tab, but it shouldn't. This behaviour prevents extensions to assign custom actions to Ctrl+Alt+number or Ctrl+Shift+number key combinations.

telega, can you test my patch on Windows and tell me whether it fixes the bug (without breaking Ctrl+number tab switching)? I'm on Mac and I need someone to test it on Windows before I request review.

I want to test, but how?

Getting Firefox building on Windows takes some effort (see http://developer.mozilla.org/en/docs/Build_Documentation). But once you get it building, just apply this patch using the "patch" utility, rebuild, and see if it fixes the bug. I think this patch is an improvement even if it doesn't fix the bug, so I'm going to request review. If it goes in, you can test a nightly to see if it fixes this bug.

Gijs verified that the patch has the desired effect on the tab-switching shortcuts on Windows, btw.

Confirming. Gavin, can you take a look at this patch? It's very small, but it fixes a rather annoying problem for people (also see the bug I just duped).

Comment on attachment 288231 [details] [diff] [review] possible patch >Index: browser/base/content/browser.js > #ifdef XP_MACOSX >- if (!event.metaKey) >+ // Mac: Cmd+number >+ if (!event.metaKey || event.ctrlKey || event.altKey || event.shiftKey) > #else > #ifdef XP_UNIX >- // don't let tab selection clash with numeric accesskeys (bug 366084) >- if (!event.altKey || event.shiftKey) >+ // Linux: Alt+number >+ if (!event.altKey || event.ctrlKey || event.metaKey || event.shiftKey) > #else >- if (!event.ctrlKey) >+ // Windows: Ctrl+number >+ if (!event.ctrlKey || event.metaKey || event.altKey || event.shiftKey) > #endif > #endif Can you factor out the common parts into a separate check rather than adding them to each conditional? It will make the pre-processed (pre-pre-processed?) source easier to read.

The only common parts are "if" and "event.shiftKey". I suppose I could replace #ifdef XP_MACOSX // Mac: Cmd+number if (!event.metaKey || event.ctrlKey || event.altKey || event.shiftKey) #else #ifdef XP_UNIX // Linux: Alt+number if (!event.altKey || event.ctrlKey || event.metaKey || event.shiftKey) #else // Windows: Ctrl+number if (!event.ctrlKey || event.metaKey || event.altKey || event.shiftKey) #endif #endif return; with if (event.shiftKey || #ifdef XP_MACOSX // Mac: Cmd+number !event.metaKey || event.ctrlKey || event.altKey) #else #ifdef XP_UNIX // Linux: Alt+number !event.altKey || event.ctrlKey || event.metaKey) #else // Windows: Ctrl+number !event.ctrlKey || event.metaKey || event.altKey) #endif #endif return; Do you want me to do that? I don't think it makes much of a difference to readability.

Oh, right. Ignore my comment, then.

Is it right just to replace the patched "browser.js" with the one in "chrome/browser.jar" or I need to go through the whole building process?

Comment on attachment 288231 [details] [diff] [review] possible patch a=beltzner

(In reply to comment #13) > Is it right just to replace the patched "browser.js" with the one in > "chrome/browser.jar" or I need to go through the whole building process? Once you've applied the patch to your source tree, you can just |make -C <objdir>/browser/base| (followed by |make -C browser/app| if you're on a Mac). If you want to manually patch your build you'll have to replace the browser.js that lives inside the browser.jar package.

(In reply to comment #13) > Is it right just to replace the patched "browser.js" with the one in > "chrome/browser.jar" or I need to go through the whole building process? browser.js needs to be preprocessed before being added to the jar file. You'll need the build process, unless you're willing to go through the code and look for lines beginning with # and preprocess them by hand... Now that this patch has an approval, it will be checked in soon, so you can test it in the first nightly after this patch.

(In reply to comment #16) > (In reply to comment #13) > > Is it right just to replace the patched "browser.js" with the one in > > "chrome/browser.jar" or I need to go through the whole building process? > > browser.js needs to be preprocessed before being added to the jar file. You'll > need the build process, unless you're willing to go through the code and look > for lines beginning with # and preprocess them by hand... Now that this patch > has an approval, it will be checked in soon, so you can test it in the first > nightly after this patch. > I went through the code and replaced //@line 1281 "/e/fx19rel/WINNT_5.2_Depend/mozilla/browser/base/content/browser.js" if (!event.ctrlKey) { //@line 1284 "/e/fx19rel/WINNT_5.2_Depend/mozilla/browser/base/content/browser.js" with //@line 1281 "/e/fx19rel/WINNT_5.2_Depend/mozilla/browser/base/content/browser.js" if (!event.ctrlKey || event.metaKey || event.altKey || event.shiftKey) { //@line 1284 "/e/fx19rel/WINNT_5.2_Depend/mozilla/browser/base/content/browser.js" Now the tab switching shortcut is not activated, but the character I was going to insert (ZWNJ using Ctrl+Shift+2) is not inserted either.

Patch checked in. If you still can't enter ZWNJ or the euro currency symbol in tomorrow morning's builds, please file a new bug report and cite the bug number here. I don't think that problem would be due to this code.

Typing the ZWNJ doesn't work with this patch as well. I'll reopen bug 414130 to track this issue. Any ideas on the possible cause?