Closed Bug 244334 Opened 21 years ago Closed 21 years ago

Text Zoom Ctrl++ broken

Categories

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

Product:
Core
Component:
DOM: UI Events & Focus Handling
Platform:
x86
Windows 98
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: hhschwab, Assigned: emaijala+moz)

Details

(Keywords: regression)

Attachments

(1 file, 2 obsolete files)

Patch v3
2.41 KB, patch
neil
: review+
roc
: superreview+
Details | Diff | Splinter Review
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a1) Gecko/20040520 Ctrl++ should increase text zoom, doesn´t using Mozilla 1.8a1 tested working using Mozilla 1.4.2 tested working Mozilla 1.8a1: Ctrl+NumPad+ increase text zoom Ctrl-- decrease text zoom Ctrl-NumPad- decrease text zoom Ctrl+0 reset text zoom to 100% not working: Bug 198233 Ctrl+NumPad 0 does not restore Text Zoom to 100%
found the function: Shift+Ctrl++ is increasing text zoom Ctrl is working on -, NumPad+, Numpad-, not on + Shift+Ctrl is working on +, NumPad+, Numpad-, not on - Also wrong documentation in help, equals sign shown instead of plus sign, 'Mozilla Keyboard Shortcuts', 'Navigator Shortcuts', 'Page Viewing Shortcuts' Zoom Text Larger Ctrl+= (plus sign) Cmd+= (plus sign) Ctrl+= (plus sign)
regressed in 1.8a, wfm 1.7 Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7) Gecko/20040514 tested working Mozilla 1.7: Ctrl++, Ctrl+NumPad+, Ctrl--, Ctrl-NumPad-, Ctrl+0 all of these combinations don´t work combined with the Shift-key, i.e. Shift+Ctrl++ is increasing text zoom filed Bug 244348 Helpfile Key Text Zoom increase should be Ctrl++, is Ctrl+= regarding the documentation.
Regression, BuildID 2004041708 working, BuildID 2004041809 failing. Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a) Gecko/20040417
Keywords: regression
Are you sure this is a keyboard bug? Does it also fail when you try to zoom through the menu option? If that also fails, then it could be a regression in layout.
(In reply to comment #4) > Are you sure this is a keyboard bug? Does it also fail when you try to zoom > through the menu option? If that also fails, then it could be a regression in > layout. It definitely is a keyboard bug, regression tests made with zips using same profile, same windows session, bug seen depending on the Mozilla version in use. Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a2) Gecko/20040522 Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a) Gecko/20040514 Firefox/0.8.0+ not working: Ctrl++ working ways to increase zoom faktor: Mozilla: View -> Text Zoom (100%) -> Larger Firefox: View -> Increase Text Size both: Shift+Ctrl++ Ctrl+Numpad+ Shift+Ctrl+Numpad+
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a) Gecko/20040417 Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a) Gecko/20040417 Firefox/0.8.0+ tested Ctrl, Shift+Ctrl, CapsLocked+CTRL, and KeyPad + Numpad: up to, including BuildID 2004041708, Mozilla + Firefox: working KeyPad: Ctrl++, Ctrl+- not working KeyPad: Shift+Ctrl++, Shift+Ctrl+- working KeyPad: CapsLock+Ctrl++, CapsLock+Ctrl+- working NumPad: Ctrl++, Ctrl+- working NumPad: Shift+Ctrl++, Shift+Ctrl+- working NumPad: CapsLock+Ctrl++, CapsLock+Ctrl+- from, including BuildID 2004041809, Mozilla + Firefox: not working KeyPad: Ctrl++, working KeyPad: Ctrl+- not working KeyPad: Shift+Ctrl+- working KeyPad: Shift+Ctrl++, not working KeyPad: CapsLock+Ctrl++ working KeyPad: CapsLock+Ctrl+- working NumPad: Ctrl++, Ctrl+- working NumPad: Shift+Ctrl++, Shift+Ctrl+- working NumPad: CapsLock+Ctrl++, CapsLock+Ctrl+- not working: Bug 198233 Ctrl+NumPad 0 does not restore Text Zoom to 100% Always working: Zoom via Menu
Does someone want to do the legwork and list the checkins during the regression window? And perhaps note the most likely to blame?
Hopefully most likely: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=SeaMonkeyAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-04-18+04%3A03&maxdate=2004-04-18+04%3A03&cvsroot=%2Fcvsroot 2004-04-18 04:03 ere%atp.fi r=neil sr=bryner Fix for bug 197474: Event.altKey && Event.ctrlKey not true together unlike IE mozilla/ widget/ src/ windows/ nsWindow.cpp 3.495 156/225 mozilla/ widget/ src/ windows/ nsWindow.h 3.172 1/2 else: Regression window: BuildID 2004-04-17 08:00 working, 2004-04-18 09:00 failing five files, 8:53 - 8:54, Bug 232175 Refactor nsNativeThemeWin http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=SeaMonkeyAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-04-18+07%3A45&maxdate=2004-04-18+09%3A50&cvsroot=%2Fcvsroot tons of Gerv´s License changes from 6:54 to 7:30 2004-04-18 07:30 2004-04-18 06:53 2004-04-18 06:53 2004-04-17 16:05 cls%seawood.org http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=SeaMonkeyAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-04-17+16%3A05&maxdate=2004-04-18+06%3A53&cvsroot=%2Fcvsroot tons of Gerv´s License changes: 2004-04-17 14:52 gerv%gerv.net 2004-04-17 14:49 gerv%gerv.net 2004-04-17 14:23 varga%nixcorp.com 2004-04-17 12:20 neil%parkwaycc.co.uk http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=SeaMonkeyAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-04-17+12%3A20&maxdate=2004-04-17+14%3A23&cvsroot=%2Fcvsroot gerv again: 2004-04-17 11:33 gerv%gerv.net 2004-04-17 09:50 gerv%gerv.net 2004-04-17 09:21 cltbld%netscape.com 2004-04-17 08:01 scott%scott-macgregor.org http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=SeaMonkeyAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-04-17+08%3A01&maxdate=2004-04-17+09%3A21&cvsroot=%2Fcvsroot Bug 240770 spell checker asserts when it has no suggestions gerv again: 2004-04-17 07:37 gerv%gerv.net 2004-04-17 07:37 gerv%gerv.net
WFM using CVS self-build from yesterday, W2K: Ctrl+Num+, Ctrl+Shift+Num+, Ctrl+=, Ctrl+"+"[Shift+=] all increase zoom Ctrl+Num-, Ctrl+Shift+Num-, Ctrl+- all decrease zoom Ctrl+0 resets zoom Note that Shift+- is "_" on my keyboard and Ctrl+_ does not decrease zoom.
WFM in CVS build as well. I'm on Windows XP. Difference between release/debug or win98/xp?
I don't think this is related to Ere's patch for bug 197474. Most of these combinations work properly for me in Firefox based on Gecko/20040522.
I´ve got a german keyboard: Q W E R T Z U I O P Ü * shifted q w e r t z u i o p ü + unshifted Y X C V B N M ; : _ shifted y x c v b n m , . - unshifted iirc US-Keyboard is ! @ # $ % ^ & * ( ) _ + ~ | shifted 1 2 3 4 5 6 7 8 9 0 - = ` \ unshifted Shift++ is * on my keyboard, so it´s no surprise that up to BuildID 2004041708 Shift#Ctrl++ doesn't increase Zoom, Ctrl++ does. But it is a surprise, that up from BuildID 2004041809 Shift#Ctrl++ does increase Zoom, Ctrl++ doesn't. I tested to nightlies, downloaded from mozilla.org, and I don´t use language packs. 1.4.2 is ok, 1.8 not. Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a2) Gecko/20040524 Ctrl++ still not working. So there is a difference between 1.4.2, and the current trunk, and that difference can be seen on the trunk if you compare official mozilla nightlies, zip builds BuildID 2004041708 working, BuildID 2004041809 failing. http://ftp26moz.newaol.com/pub/mozilla.org/mozilla/nightly/2004-04-17-09-trunk/mozilla-i586-pc-msvc.zip http://ftp26moz.newaol.com/pub/mozilla.org/mozilla/nightly/2004-04-18-10-trunk/
Please can you try Dean's Keyboard Test Page which is attachment 55849 [details]
Ctrl++: keydown charCode=0 keyCode=61 character=| keypress charCode=93 keyCode=0 character=|]| modifiers=Ctrl keyup charCode=0 keyCode=61 character=| Shift+Ctrl++ keydown charCode=0 keyCode=61 character=| keypress charCode=61 keyCode=0 character=|=| modifiers=Ctrl+Shift keyup charCode=0 keyCode=61 character=| Shift+Ctrl++: all 3 lines had the 'modifiers=Ctrl+Shift' shown, but it wasn´t pasted here. I also couldn´t paste (or copy?) all three lines, had to do it line by line.
Aha! On German keyboards Ctrl++ actually produces a ^] - telnet thus advertises it as the "escape character", whereas on US keyboards it advertises Ctrl+]. Now Windows actually sends ^] as a WM_CHAR of value 29. Prior to ere's fix we were getting multiple key events, one synthetic one for the Ctrl+keyCode and one real one for the ^]. 1) keydown charCode=0 keyCode=61 character=|| modifiers=Ctrl 2) keypress charCode=61 keyCode=0 character=|=| modifiers=Ctrl 3) keypress charCode=93 keyCode=0 character=|]| modifiers=Ctrl 4) keyup charCode=0 keyCode=61 character=|| modifiers=Ctrl Subsequent to ere's fix we don't synthesize the Ctrl+keyCode because we see that Windows has generated a WM_CHAR event. Windows uses Ctrl++ for the ^] char because ] is already Ctrl+Alt+9.
Assignee: aaronleventhal → ere
Great. I believe this has been broken at some moment in the past too. Not a suprise considering the Windows' behavior. I also see the problem with the Finnish keyboard. I'll try to fix it.
Attached patch Patch v1 (obsolete) — — Splinter Review
This patch makes it so that Ctrl + NS_VK_ADD or NS_VK_SUBTRACT is always handled separately. Could someone with German keyboard test it? I wonder if there is a need to add NS_VK_MULTIPLY and NS_VK_DIVIDE to this special case.
Comment on attachment 149673 [details] [diff] [review] Patch v1 I've tested the patch with today's source and it does *not* work for my German keyboard and layout. >+ if (virtualKeyCode == NS_VK_RETURN || virtualKeyCode == NS_VK_BACK || >+ virtualKeyCode == NS_VK_ADD || virtualKeyCode == NS_VK_SUBTRACT) If I hit CTRL++, I get here with virtualKeyCode == 0x3d ('='), and nothing happens wrt to zooming text (CTRL+- produces virtualKeyCode == NS_VK_SUBTRACT). FWIW: For CTRL++, OnKeyDown is called with aVirtualKeyCode == 0xbb, aScanCode == 0x1b and aKeyData == 0x1b0001.
JFTR: For CTRL+-, OnKeyDown is called with aVirtualKeyCode == 0xbd, aScanCode == 0x35 and aKeyData == 0x350001.
WFM with the german keyboard and Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7) Gecko/20040601 Firefox/0.8.0+ Karsten could you test again with the current build?
Henrik, this patch hasn't even been reviewed let alone checked in - and so, not surprisingly, it does not work with Mozilla 1.8a2 [Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a2) Gecko/20040601]. Maybe it's fixed in FF, but definitely *not* in Mozilla.
Attachment #149673 - Attachment is obsolete: true
Karsten you are right. I forgot that i use the aviary branch of FF now. Cause it based on 1.7 i can't see this bug there. So forget comment 21.
I'm still pondering what to do with this (other than resort to the old way which had its own problems).
Attached patch Patch v2 (obsolete) — — Splinter Review
This patch adds even more specially handled keys. It should fix the German keyboard and Ctrl+NumPad0 (restore 100%). Now all the layouts I tried generate Ctrl+= when pressing Ctrl++, which is still less than perfect but works. Could you guys try the patch?
It does now work for me. The keyboard test page (see comment #14) reports = as the character for CTRL+(Shift+)+'+' 0 as the character for CTRL+(Shift+)+'0' and CTRL+NUMPAD_0 - as the character for CTRL+'-' The part + (mIsControlDown && !mIsAltDown && !mIsShiftDown && + (virtualKeyCode == NS_VK_ADD || virtualKeyCode == NS_VK_SUBTRACT || + virtualKeyCode == NS_VK_EQUALS || virtualKeyCode == NS_VK_NUMPAD0))) looks quite "hackish", though - are these four keys the only ones to generate this problem? Or do we need to dig up this hole everytime we start using certain keys as shortcuts?
Not sure if this link is helpful, it's a great reference on international keyboard layouts. http://www.macromedia.com/support/fontographer/ts/documents/master.htm Unfortunately you assume much about punctuation key placement without checking all of these kwyboard diagrams. Different languages have competely different punctuation key placement. Punctuation which is on different keys for English keyboards might be on the same key on a Polish keyboard, or vice versa.
It seems kind of hacky to me, but I don't know how else to handle it right now. Do we need to special case NumPad0, though?
Comment on attachment 151798 [details] [diff] [review] Patch v2 >+ virtualKeyCode == NS_VK_EQUALS || virtualKeyCode == NS_VK_NUMPAD0))) I don't think you need to special case DOM_VK_NUMPAD0 here, as Windows will already generate the correct WM_CHAR of '0' in the unmodified numlock case. >+ case NS_VK_NUMPAD0 : asciiKey = '0'; break; I think that in order to be consistent you should special case the entire range DOM_VK_NUMPAD0 to DOM_VK_NUMPAD9 and also DOM_VK_DECIMAL.
True, this is hackish, but I don't see any other way to fix this. In all locales I've seen the problem in, the WM_CHAR message Windows generates for Ctrl++ is something unexpected. That's why we need to bypass it and handle those cases ourselves. It's true that this may need to be revisited if Ctrl+something else ill-behaving is used as a shortcut, but I think there are not so many good shortcuts left and there's always the possibility they work fine :) > Do we need to special case NumPad0, though? You and Neil are right, we don't. I'll remove that. > think that in order to be consistent you should special case the entire range > DOM_VK_NUMPAD0 to DOM_VK_NUMPAD9 and also DOM_VK_DECIMAL. True, I'll add them except decimal, which isn't worth it digging out (I tried GetLocaleInfo(GetThreadLocale()...) but it returns wrong information if you change the keyboard), unless you insist.
Status: NEW → ASSIGNED
Attached patch Patch v3 — — Splinter Review
Patch including suggested changes.
Attachment #151798 - Attachment is obsolete: true
Attachment #151900 - Flags: review+
Attachment #151900 - Flags: superreview?(bryner)
Comment on attachment 151900 [details] [diff] [review] Patch v3 + // Ctrl+[Add, Subtract, Equals] are always handled here to make text zoom shortcuts work + // on different keyboard layouts. Can you explain why we have to handle Equals in the comment?
Ok, I'll add that to the comment.
Attachment #151900 - Flags: superreview?(bryner) → superreview?(roc)
Attachment #151900 - Flags: superreview?(roc) → superreview+
Fix checked in to trunk.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
v.
Status: RESOLVED → VERIFIED
Component: Keyboard: Navigation → User events and focus handling
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: