| Anonymous | Login | 01-17-2020 18: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 | ||||||||
| 0006930 | [Squeak Packages] Polymorph | major | sometimes | 02-16-08 16:51 | 02-19-08 16:18 | ||||||||
| Reporter | dr | View Status | public | ||||||||||
| Assigned To | GazzaGuru | ||||||||||||
| Priority | normal | Resolution | fixed | ||||||||||
| Status | resolved | ||||||||||||
| Summary | 0006930: maximumSelection gives wrong results in PluggableListMorph | ||||||||||||
| Description |
We noticed this problem in the latest squeak-dev image based on 3.10 using latest OmniBrowser. Pinesoft-Widgets-gvc.280 is installed which overrides a couple of methods in PluggableListMorph, also #maximumSelection. Sometimes (hard to find out when exactly) when we browse in OmniBrowser methods in a pluggable list and use key down and up, we get a debugger because maximumSelection answers a value bigger than the number of elements in the list. Also the inst var 'list' in PluggableListMorph has the wrong entries (ie. methods), often those of the class we browsed before the one we are browsing now, so if this first class has more methods that the current one, maximumSelection returns an index number not included in the current method list. It seems that 'list' is not probperly updated. Eveything works again when we switch back to the original version of #maximumSelection which does not use a cache. I don't think that this problem is related to OmniBrowser, the same might occur somewhere else as well. |
||||||||||||
| Additional Information | |||||||||||||
| Attached Files | |||||||||||||
|
|
|||||||||||||
 Relationships |
|
|
Notes |
|
|
(0011822 - 112 - 122 - 122 - 122 - 122 - 122) GazzaGuru 02-18-08 10:15 |
Looks like I haven't tested throughly in the "indirect" case (using getListSizeSelector etc.). Will investigate. |
|
(0011823 - 315 - 321 - 321 - 321 - 321 - 321) GazzaGuru 02-18-08 10:41 |
Having looked it seems that it may be a timing issue of some kind. Perhaps the selected item of the list is being triggered before the list change notification (in OB). Previously the call for maximumSelection (called via selectionIndex: possibly) would get the list regardless (very time consuming in some cases). |
|
(0011826 - 60 - 60 - 60 - 60 - 60 - 60) GazzaGuru 02-18-08 11:40 |
It would be helpful to have a reproduceable case for this... |
|
(0011827 - 236 - 254 - 410 - 410 - 410 - 410) dr 02-18-08 14:24 |
Here is an image in which you can see the problem: http://scg.iam.unibe.ch/Research/Hermion/BrokenOB.zip [^] A debugger is already opened. You can reproduce the problem again if you select the method #wheelSpeeds and hit the down arrow. |
|
(0011829 - 21 - 21 - 21 - 21 - 21 - 21) GazzaGuru 02-18-08 15:06 |
Ta. Will take a look. |
|
(0011830 - 376 - 398 - 398 - 398 - 398 - 398) GazzaGuru 02-18-08 15:25 |
It appears that the "damage" occurred some time before the error. Apparently the OB node structure had changed without the list getting updated. The old behaviour of maximumSelection would have re-requested the list (not causing an error), but not updated the display, I think, anyway. It seems clear, to me, that this is a problem with OB synchronising with the list view. |
|
(0011831 - 371 - 383 - 383 - 383 - 383 - 383) dr 02-18-08 15:30 |
Well, the display gets updated properly, because for displaying the list morph asks the model, but for fetching the maximum size of the list, it does not. This seems strange to me. What would be the correct way to update the list from OB? PluggableListMorph was designed to make the model responsible for holding the list elements and being up-to-date, not vice-versa. |
|
(0011832 - 509 - 529 - 529 - 529 - 529 - 529) GazzaGuru 02-18-08 16:54 |
As I implied, the list (on last change/update) seems to have the "old" list of methods, apparently that no longer matched up with the metanode structure held by OB. Normally you would expect these to be in sync (if OB notifies via the change mechanism that the underlying model has changed). I think something is amiss with the change notification on the OB side (or, perhaps, the system change notifications for methods etc. has been "unlinked"). From a fresh image I am unable to reproduce these symptoms... |
|
(0011837 - 1698 - 1785 - 1785 - 1785 - 1785 - 1785) dr 02-19-08 15:30 |
yes, I also noticed that the method list stored in the 'list' inst var contains the methods from the previously selected class. But OB does notify the list with an update announcement. The problem is a tad more complex. Look how the 'list' inst var is set in PluggableListMorph. There is only one method writing to 'list', namely #getList. (except #list:, but this is deprecated). Now look who is invoking #getList: (i) #selection: - never invoked in that image (ii) #verifyContents - never invoked when using OB (iii) #getListSize - but only if 'getListSizeSelector' is not specified (not in our case) (iv) #getListItem: - but only if 'getListElementSelector' is not specified (not in our case) (v) #copyListToClipboard - not relevant (vi) #basicKeyPressed: - yes, this is the only method invoking #getList and thus setting the 'list' var. Now do the following: Go to eg. class Boolean using OB, hit a letter to select a method of Boolean starting with this letter (#basicKeyPressed: is invoked). Then go to class False (has less methods than Boolean), select a method and press the arrow key until the end of this list and one more -> you will get the error. You see, you cannot rely that the 'list' inst var contains the right list unless you pressed a key in that list, because then #basicKeyPressed: is invoked, the only method setting a value to 'list' (when 'getListElementSelector' and 'getListSizeSelector' are specified)... So this change to #maximumSelection is suspicious. As I said, PluggableListMorph is intented to ask the model for list elements, not to hold the list itself. If you want to change this intention, a lot of code would need adaptions, not only OB. |
|
(0011838 - 489 - 507 - 507 - 507 - 507 - 507) GazzaGuru 02-19-08 15:59 |
Unfortunately, things are more complicated than that. PluggableListMorph uses a (unhealthy, in my opinion)LazyListMorph to hold its contents as well as deliberately caching the list items... see PluggableListMorph>>getlistItem: and senders. In fact, for an up/down key, #specialKeyPressed; is called which implicitly requires #maximumSelection to have updated the list. The whole PluggableListMorph is really evil and should be simplified. In the meantime I'll revert #maximumSelection. |
|
(0011839 - 58 - 58 - 58 - 58 - 58 - 58) GazzaGuru 02-19-08 16:00 |
If you would like to test with Pinesoft-Widgets-gvc.290... |
|
(0011840 - 576 - 594 - 594 - 594 - 594 - 594) dr 02-19-08 16:18 |
I tested again with this version and it seems to work, cannot reproduce it anymore, thanks! yes, I agree, PluggableListMorph is evil... Well, in that case the LazyListMorph was not directly involved. It was a very rare problem: Only if you invoked #basicKeyPressed: in another list with less elements and then update the list without pressing a basic key, but a special key which required 'list' to be updated in the meantime, you got that problem. Very rare situation. We could already improve the situation to make sure that 'list' is either used nowhere or everywhere. |
|
Issue History |
|||
| Date Modified | Username | Field | Change |
| 02-16-08 16:51 | dr | New Issue | |
| 02-16-08 16:51 | dr | Status | new => assigned |
| 02-16-08 16:51 | dr | Assigned To | => GazzaGuru |
| 02-18-08 10:15 | GazzaGuru | Note Added: 0011822 | |
| 02-18-08 10:41 | GazzaGuru | Note Added: 0011823 | |
| 02-18-08 11:40 | GazzaGuru | Note Added: 0011826 | |
| 02-18-08 14:24 | dr | Note Added: 0011827 | |
| 02-18-08 15:06 | GazzaGuru | Note Added: 0011829 | |
| 02-18-08 15:25 | GazzaGuru | Note Added: 0011830 | |
| 02-18-08 15:30 | dr | Note Added: 0011831 | |
| 02-18-08 16:54 | GazzaGuru | Note Added: 0011832 | |
| 02-19-08 15:30 | dr | Note Added: 0011837 | |
| 02-19-08 15:59 | GazzaGuru | Note Added: 0011838 | |
| 02-19-08 16:00 | GazzaGuru | Note Added: 0011839 | |
| 02-19-08 16:00 | GazzaGuru | Status | assigned => feedback |
| 02-19-08 16:18 | dr | Status | feedback => resolved |
| 02-19-08 16:18 | dr | Resolution | open => fixed |
| 02-19-08 16:18 | dr | Note Added: 0011840 | |
| Mantis 1.0.8[^]
Copyright © 2000 - 2007 Mantis Group
81 total queries executed. 52 unique queries executed. |  |
