Closed
Bug 306686
Opened 19 years ago
Closed 19 years ago
/ does not start FAYT anymore
Categories
(SeaMonkey :: Find In Page, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: Biesinger, Assigned: aaronlev)
References
Details
(Keywords: helpwanted, regression)
Attachments
(2 files)
(deleted),
patch
|
aaronlev
:
review+
neil
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
bzbarsky
:
review+
neil
:
superreview+
|
Details | Diff | Splinter Review |
checkout finish: Mi Aug 31 22:11:43 CEST 2005
Linux gtk2 seamonkey, trunk
Pressing / does not have any effect. That means I have no way to easily start
typeaheadfind for text (as opposed to links).
It may be worth nothing that / is Shift+7 on my keyboard. So maybe this is
caused by bug 295228?
Reporter | ||
Updated•19 years ago
|
Keywords: regression
Comment 1•19 years ago
|
||
With today's SeaMonkey trunk build on Win32 the / starts the FAYT. Could it be
GTK2 widget code that causes problems?
For me pressing / the nsXBLPrototypeHandler::KeyEventMatched receives the code
0x2F ('/') and applying ToLowerCase to it does not change anything.
Comment 2•19 years ago
|
||
Actually it could be a problem. GTK widget code reports that Shift key is down
and match in nsXBLPrototypeHandler::ModifiersMatchMask fails. Now that we take
into account Shift key state it becomes important.
It is problem of widget code. If modifier keys (Shift, Alt, Ctrl, Meta) together
with other key produce different character than in non shifted state then widget
code should NOT report this modifier key. In your case Shift+7 should only
report '/' and not Shift+'/'.
This is very similar to Win32 specific widget problem that I've fixed in bug 287179.
Reporter | ||
Comment 3•19 years ago
|
||
ok, then -> widget:gtk, I guess
Assignee: aaronleventhal → blizzard
Component: Keyboard: Find as you Type → Widget: Gtk
Flags: blocking1.9a1?
QA Contact: keyboard.fayt → gtk
Comment 4•19 years ago
|
||
Actually I can recreate the same problem on Windows if I install the German
keyboard layout :(
In previous note when I was talking about not reporting Shift state I was
thinking that you refer to 7 on your numerical keyboard. About '7' in keyboard
first row I am not so sure. I guess that Shift have to be reported for backwards
compatibility.
Is it acceptable to add duplicate rules in XBL that have Shift modifier declared
for those shortcuts for which case is not important?
Component: Widget: Gtk → Keyboard: Find as you Type
OS: Linux → All
Reporter | ||
Comment 5•19 years ago
|
||
modifiers="any"?
Reporter | ||
Comment 6•19 years ago
|
||
(hmm, does that exist?)
Comment 7•19 years ago
|
||
That exists in XBL but this is a XUL key which doesn't really support it
c.f. the Zoom In key which is both Ctrl++ and Ctrl+Shift++ just in case.
Comment 8•19 years ago
|
||
When I used:
<handler event="keypress" key="/" command="cmd_findTypeText" modifiers="shift any"/>
in content/xbl/builtin/browser-base.inc
and:
<key id="key_findTypeText"
key="&findTypeTextCmd.key;" modifiers="shift any"/>
<key id="key_findTypeLinks"
key="&findTypeLinksCmd.key;" modifiers="shift any"/>
in xpfe/communicator/resources/content/win/platformCommunicatorOverlay.xul
the FAYT works as expected with German keyboard layout.
The problem is that ' modifiers="shift any" ' should be added for both XPFE and
Toolkit for all key non character key acceleretors. Is there some centralized
place where can I find all shortcut combinations used by both toolkits?
Comment 9•19 years ago
|
||
I've meant non letter characters.
Updated•19 years ago
|
Assignee: blizzard → aaronleventhal
QA Contact: gtk → keyboard.fayt
Assignee | ||
Comment 11•19 years ago
|
||
Does anyone know what caused this regression?
Comment 12•19 years ago
|
||
Assignee | ||
Updated•19 years ago
|
Keywords: helpwanted
Comment 13•19 years ago
|
||
Luckily there were not as many non-letter shortcut combinations that are
handled by XBL bindings as I thought initially.
By inspecting both Firefox and Mozilla help files seems that there are only
those combinations:
/,' - FAYT
Ctrl++, Ctrl--, Cmd++, Cmd+-
Cmd+.
And only FAYT shortcuts are in XBL. As Neil said the "any" is supported only
for XBL and not for XUL. Because bug 295228 changed only XBL behaviour, the XUL
shortcuts should work fine.
Seems that it is sufficient to change only content/xbl/builtin/browser-base.inc
to fix the problem.
Updated•19 years ago
|
Attachment #194568 -
Flags: review?(neil.parkwaycc.co.uk)
Comment 14•19 years ago
|
||
Comment on attachment 194568 [details] [diff] [review]
Fix for FAYT shortcut keys
I can't test this but it looks reasonable.
Attachment #194568 -
Flags: superreview+
Attachment #194568 -
Flags: review?(neil.parkwaycc.co.uk)
Attachment #194568 -
Flags: review?(aaronleventhal)
Assignee | ||
Comment 15•19 years ago
|
||
I can't find any docs on the "any" option of the modifiers attribute.
It looks like we're saying shift + another modifier is required.
Also, I have seen some times there are commas in between modifiers.
Can someone point me to a document that explains this or just write it into this
bug or to the developer.mozilla.org wiki? We need to get XUL planet to update
their docs too.
I can't r= this until I understand that.
Comment 16•19 years ago
|
||
Have not seen documentation for "any", but you can look at the source code of
nsXBLPrototypeHandler::ConstructPrototype
http://lxr.mozilla.org/seamonkey/source/content/xbl/src/nsXBLPrototypeHandler.cpp#863
Everything that is before "any" is ignored by clearing the mask bits in higher
nybble in mKeyMask.
See for usage samples in:
xpfe\global\resources\content\bindings\listbox.xml
xpfe\global\resources\content\bindings\tree.xml
Thus "control shift any" means that state of Ctrl and Shift keys is not
important, but Alt and Meta should not be pressed.
The comma is valid separator at least for XBL - see the
nsXBLPrototypeHandler::ConstructPrototype again:
char* token = nsCRT::strtok( str, ", \t", &newStr );
I fully agree that XulPlanet docs should be updated to match reality.
Assignee | ||
Comment 17•19 years ago
|
||
Comment on attachment 194568 [details] [diff] [review]
Fix for FAYT shortcut keys
Thanks for the explanation. I'll ping XUL Planet with it.
Attachment #194568 -
Flags: review?(aaronleventhal) → review+
Comment 18•19 years ago
|
||
This is strictly speaking not related to the problem of FAYT, but it would be
nice to have XUL "modifiers" to be consistent with XBL modifiers and allow not
only space and comma as separators, but also the TAB character.
Attachment #194653 -
Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #194653 -
Flags: review?(aaronleventhal)
Assignee | ||
Comment 19•19 years ago
|
||
Comment on attachment 194653 [details] [diff] [review]
Additional patch to allow TAB as separator for XUL nsMenuFrame
I'm not the right person to review this -- try a layout peer.
Also, we really need to file a separate bug on this issue and post the patch in
there. Something like "Synchronize XUL and XBL handler syntax"
Attachment #194653 -
Flags: review?(aaronleventhal)
Updated•19 years ago
|
Attachment #194653 -
Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
Updated•19 years ago
|
Attachment #194653 -
Flags: review?(bzbarsky)
Updated•19 years ago
|
Attachment #194653 -
Flags: review?(bzbarsky) → review+
Comment 20•19 years ago
|
||
Neil, are you going to check these in?
I won't be able to until Tuesday; if they're not landed by then, please ping me?
Reporter | ||
Comment 21•19 years ago
|
||
looks like this was checked in at 2005-09-02 08:54 on trunk
Comment 22•19 years ago
|
||
*** Bug 306639 has been marked as a duplicate of this bug. ***
Updated•19 years ago
|
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Product: Core → SeaMonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•