Anonymous | Login | 04-12-2021 16:37 UTC |
Main | My View | View Issues | Change Log | Docs |
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 |
![]() ![]() ![]() |
||||||||
|
![]() |
|||||||||||||||||||||||||
SYSTEM WARNING: Creating default object from empty value SYSTEM WARNING: Creating default object from empty value SYSTEM WARNING: Creating default object from empty value
|
![]() |
|
(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. |
Mantis 1.0.8[^]
Copyright © 2000 - 2007 Mantis Group
147 total queries executed. 80 unique queries executed. |