Mantis Bugtracker
  

Viewing Issue Simple Details Jump to Notes ] View Advanced ] Issue History ] Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0006462 [Squeak] Morphic minor always 05-03-07 16:44 11-17-08 21:41
Reporter dpollet View Status public  
Assigned To
Priority normal Resolution fixed  
Status closed   Product Version 3.10
Summary 0006462: Jump-by-key in some lists broken
Description PluggableListMorph>>basicKeyPressed: is responsible for the "jump to an item by typing its first letter(s)" feature in lists, which is broken in e.g. the class column of Omnibrowser.
This is because of this identity comparison near the end of the method, which should actually be a simple equality so it still works with Texts or Strings instead of Symbols:
        nextSelection := self getList findFirst: [:a | a == nextSelectionText].

Additionally the jump behavior is wrong when typing several keystrokes in a burst to jump to a multi-letter-prefix: it will jump to the next matching item at each keystroke within a keystroke burst, instead of staying in place as long as the item still matches the typed prefix.

Here is a fixed implementation (tested in OmniBrowser and the OB Universe Browser, on a 3.10 squeak-dev image):
basicKeyPressed: aChar
    | oldSelection nextSelection max milliSeconds slowKeyStroke nextSelectionList nextSelectionText |
    nextSelection := oldSelection := self getCurrentSelectionIndex.
    max := self maximumSelection.
    milliSeconds := Time millisecondClockValue.
    slowKeyStroke := milliSeconds - lastKeystrokeTime > 300.
    lastKeystrokeTime := milliSeconds.
    nextSelectionList := OrderedCollection newFrom: (self getList copyFrom: oldSelection + 1 to: max).
    nextSelectionList addAll: (self getList copyFrom: 1 to: oldSelection - 1).
    slowKeyStroke
        ifTrue: ["forget previous keystrokes and search in following elements"
            lastKeystrokes := aChar asLowercase asString.
            nextSelectionList addLast: (self getList at: oldSelection)]
        ifFalse: ["append quick keystrokes but don't move selection if it still matches"
            lastKeystrokes := lastKeystrokes , aChar asLowercase asString.
            nextSelectionList addFirst: (self getList at: oldSelection)].
    "Get rid of blanks and style used in some lists"
    nextSelectionText := nextSelectionList
        detect: [:a | a asString withBlanksTrimmed asLowercase beginsWith: lastKeystrokes]
        ifNone: [^ self flash "match not found"].
    "No change if model is locked"
    model okToChange ifFalse: [^ self].
    nextSelection := self getList findFirst: [:a | a = nextSelectionText].
    oldSelection == nextSelection ifTrue: [^ self flash].
    ^ self changeModelSelection: nextSelection
Additional Information
Attached Files  PluggableListMorph-basicKeyPressed.cs [^] (1,581 bytes) 05-03-07 20:32
 PluggableListMorphFix.1.cs [^] (1,647 bytes) 05-24-07 11:34
 PluggableListMorphFix2-M6462.st [^] (2,027 bytes) 11-28-07 18:36

- Relationships

SYSTEM WARNING: Creating default object from empty value

SYSTEM WARNING: Creating default object from empty value

SYSTEM WARNING: Creating default object from empty value

related to 0006552new  Squeak in 7108 typing in a file pane causes a error 
related to 0006788resolved GazzaGuru Squeak Packages Bug with an Empty PluggableListMorph 
has duplicate 0007207closed Damien Cassou Squeak Packages Primitive failed while jumping to method by keyboard 
child of 0006635new  Squeak Mother of Squeak UI annoyances 

- Notes
(0010703 - 126 - 132 - 132 - 132 - 132 - 132)
edgardec
05-11-07 20:53

For include this into 3.10, I wish a 3.10 base example or what Damien Cassou said this is a good fix.
A test case could help.
 
(0010704 - 126 - 132 - 132 - 132 - 132 - 132)
edgardec
05-11-07 20:53

Reminder sent to: dpollet

For include this into 3.10, I wish a 3.10 base example or what Damien Cassou said this is a good fix.
A test case could help.
 
(0010712 - 95 - 95 - 95 - 95 - 95 - 95)
Damien Cassou
05-12-07 22:38

I can't guarantee, but the fix seems ok to me. And writing a test for it does seem feasible :-)
 
(0010752 - 127 - 127 - 127 - 127 - 127 - 127)
dpollet
05-24-07 11:36

Attached a new version of the method (PluggableListMorphFix.1.cs) that includes a fix for the selection bounds by Andrew Black.
 
(0010754 - 100 - 106 - 106 - 106 - 106 - 106)
edgardec
05-24-07 18:10

This now is 7112PluggableListMorph-basicKeyPressed..cs and was in updates for 3.10
Thanks Damien !
 
(0010854 - 358 - 394 - 394 - 394 - 394 - 394)
wiz
07-03-07 02:44
edited on: 07-03-07 06:12

The latest fix (7112PluggableListMorph-basicKeyPressed.cs) did NOT get into the image (7121) the corresponding changeset MC30 is empty. This problem is similar to the problem that first occured with 7109. Repository woes are suspected.



There is still a very bad smell to the three routines that use this code see the other report for further details

 
(0010887 - 75 - 75 - 75 - 75 - 75 - 75)
Damien Cassou
07-17-07 13:52

Please try again to include the fix provided by PluggableListMorphFix.1.cs.
 
(0010900 - 143 - 143 - 143 - 143 - 143 - 143)
wiz
07-20-07 00:08

In squeak 7122beta image the fix is still not in. Can't tell if the cs was empty or not. The breadcrumbs (changeset records) have been removed.
 
(0010901 - 451 - 517 - 517 - 517 - 517 - 517)
wiz
07-20-07 03:33

'apb 5/23/2007 14:16 PluggableListMorph basicKeyPressed: {model access}' This is the one that fixes the bug
PluggableListMorphFix.1.cs


'cdlm 5/3/2007 18:28 PluggableListMorph basicKeyPressed: {model access}' this is the one that causes the bug.
PluggableListMorph-basicKeyPressed.cs

The second is in the 7122 image. The former is not in the image at all.

Did someone update with the wrong change?

Yours in curiosity and service, -Jer
 
(0010902 - 328 - 352 - 352 - 352 - 352 - 352)
wiz
07-20-07 03:46

I note the the Morphic-edc packages 123 and 124 are both dealing with this bug according to the comment. So I'm wondering now if the reason that the proper fix did not get in was because 124 loaded the same (wrong) fix as 123?

How does the release process pick a fix to load?

Yours in curiosity and service, --Jerome Peace
 
(0010903 - 85 - 97 - 97 - 97 - 97 - 97)
wiz
07-20-07 03:47

Reminder sent to: edgardec

Did some more tracking. You might have the final piece of the puzzle.

Cheers, -Jer
 
(0011486 - 398 - 469 - 469 - 469 - 469 - 469)
dpollet
11-28-07 17:44

This is still broken in OB from squeak-dev based on 7154:
- open an OB window
- move the mouse over one of the columns (usually the package list)
- press a key => Primitive failed

This statement is incorrect when there is no selection:
    nextSelectionList addAll: (self getList copyFrom: 1 to: oldSelection - 1).

oldSelection is 0 so copyFrom:to: tries to allocate an Array with size -1
 
(0011487 - 9 - 9 - 9 - 9 - 9 - 9)
dpollet
11-28-07 17:44

reopening
 
(0011488 - 136 - 148 - 148 - 148 - 148 - 148)
dpollet
11-28-07 18:42

Attached PluggableListMorphFix2-M6462.st

This merges Andrew's changes into whatever is in 7154, plus another fix by me. Please checků
 
(0011491 - 125 - 125 - 253 - 253 - 253 - 253)
dpollet
11-28-07 20:15

OK it seems the fix actually is in 3.10 but OB overrides it with a buggy version; see http://bugs.squeak.org/view.php?id=6788 [^]
 
(0011494 - 22 - 22 - 22 - 22 - 22 - 22)
Damien Cassou
11-29-07 07:29

Are you sure it's OB ?
 
(0012664 - 466 - 490 - 490 - 490 - 490 - 490)
KenCausey
09-15-08 20:52

PluggableListMorph-basicKeyPressed.cs harvested in update 7108. Update 7112 refers to this issue but contains nothing but an array of class and method removals and it's relation to this issue is unclear. PluggableListMorphFix.1.cs harvested in update 7130. Both of these released in 3.10.

PluggableListMorphFix2-M6462.st has not been harvested as of 3.10.2-7179.

Is it still felt that this fix provides some value? It's hard to tell from the description.
 
(0012666 - 107 - 107 - 107 - 107 - 107 - 107)
dpollet
09-16-08 08:07

No, I think it's OK. I still get the bug from time to time but it's due to OB indenting list items I guess.
 
(0012776 - 139 - 139 - 139 - 139 - 139 - 139)
KenCausey
11-17-08 21:41

Harvested in update 7108 released in 3.10. Any remaining issue suspected to be related to Omnibrowser specifically and reported elsewhere.
 

- Issue History
Date Modified Username Field Change
05-03-07 16:44 dpollet New Issue
05-03-07 20:32 dpollet File Added: PluggableListMorph-basicKeyPressed.cs
05-11-07 20:53 edgardec Note Added: 0010703
05-11-07 20:53 edgardec Note Added: 0010704
05-12-07 22:38 Damien Cassou Note Added: 0010712
05-24-07 11:34 dpollet File Added: PluggableListMorphFix.1.cs
05-24-07 11:36 dpollet Note Added: 0010752
05-24-07 18:10 edgardec Note Added: 0010754
07-01-07 18:27 wiz Relationship added related to 0006552
07-03-07 02:44 wiz Note Added: 0010854
07-03-07 02:45 wiz Status new => resolved
07-03-07 02:45 wiz Resolution open => fixed
07-03-07 06:12 wiz Note Edited: 0010854
07-03-07 06:12 wiz Status resolved => feedback
07-03-07 06:12 wiz Resolution fixed => reopened
07-17-07 13:52 Damien Cassou Note Added: 0010887
07-20-07 00:08 wiz Note Added: 0010900
07-20-07 03:33 wiz Note Added: 0010901
07-20-07 03:46 wiz Note Added: 0010902
07-20-07 03:47 wiz Issue Monitored: edgardec
07-20-07 03:47 wiz Note Added: 0010903
08-22-07 04:29 matthewf Relationship added child of 0006635
10-10-07 10:29 edgardec Status feedback => resolved
11-28-07 17:44 dpollet Note Added: 0011486
11-28-07 17:44 dpollet Status resolved => feedback
11-28-07 17:44 dpollet Note Added: 0011487
11-28-07 18:36 dpollet File Added: PluggableListMorphFix2-M6462.st
11-28-07 18:42 dpollet Note Added: 0011488
11-28-07 20:15 dpollet Note Added: 0011491
11-29-07 03:01 wiz Relationship added related to 0006788
11-29-07 07:29 Damien Cassou Note Added: 0011494
09-15-08 20:52 KenCausey Note Added: 0012664
09-16-08 08:07 dpollet Note Added: 0012666
10-07-08 05:24 Damien Cassou Relationship added has duplicate 0007207
11-17-08 21:41 KenCausey Status feedback => closed
11-17-08 21:41 KenCausey Note Added: 0012776
11-17-08 21:41 KenCausey Resolution reopened => fixed
11-17-08 21:41 KenCausey Fixed in Version  => 3.10


Mantis 1.0.8[^]
Copyright © 2000 - 2007 Mantis Group
147 total queries executed.
80 unique queries executed.
Powered by Mantis Bugtracker