Closed Bug 244334 Opened 20 years ago Closed 20 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: 20 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: