20 Bug reports/feature suggestions/fixes for Dolphin 5.1.3

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

20 Bug reports/feature suggestions/fixes for Dolphin 5.1.3

Maxim Fridental
Blair,

thank you very much for your support. We would like to report some other
bugs we've found. Our development environment is Dolphin Pro 5.1.3 under
Windows 2000 Terminal Server. Note, that we have already posted some
problems in the newsgroup, but received no answer.


1) Problem with beSorted: (ListView replaces the old collection in the
ListModel with a new sorted collection). Recorded by Andy Bower as defect
#1134. One possible solution is to extend the ListModel, so that is also get
its list from a ValueModel. The code below has a lot if #isKindOf:
statements, because we didn't want to break existing ListModels. The fix for
the VideoLibrary is also included.

!ListModel methodsFor!

list
 "Answer the contents of the receiver"

 ^(list isKindOf: ValueModel) ifTrue: [list value] ifFalse: [list]! !
!ListModel categoriesFor: #list!accessing!public! !

!ListModel methodsFor!

list: aSequenceableCollection
 "Set the list of the receiver, i.e. the collection that it represents"

 self list == aSequenceableCollection
  ifFalse:
   [(list isKindOf: ValueModel)
    ifTrue: [list value: aSequenceableCollection]
    ifFalse: [list := aSequenceableCollection].
   self notifyListChanged]! !
!ListModel categoriesFor: #list:!accessing!public! !

!ListModel methodsFor!

listHolder
 ^(list isKindOf: ValueModel) ifTrue: [list] ifFalse: [list := list
asValue]! !
!ListModel categoriesFor: #listHolder!accessing!public! !

!ListModel methodsFor!

listHolder: aValueModel
 list := aValueModel.
 self notifyListChanged! !
!ListModel categoriesFor: #listHolder:!accessing!public! !

!ListModel methodsFor!

postCopy
 "Apply any final flourish to the copy that may be required
 in order to ensure that the copy does not share any state with
 the original, apart from the elements. Answer the receiver.
 In the case of a ListModel we need to ensure the contained collection
 is copied too, and the copy has no event registrations."

 super postCopy.
 list := self list copy.
 events := nil! !
!ListModel categoriesFor: #postCopy!copying!public! !

!ListModel methodsFor!

setList: collection searchPolicy: policy
 "Private - Initialize the receiver's identity instance variables. Answer
the receiver."

 list := collection.
 searchPolicy := policy! !
!ListModel categoriesFor: #setList:searchPolicy:!initializing!private! !

!VideoLibraryShell methodsFor!

onTapeSelected
 "Private - Event handler for when a new tape has been selected. Transfer
the
 recordings into the recordingsPresenter."

 self hasSelectedTape
  ifTrue: [recordingsPresenter model listHolder: (self tape aspectValue:
#recordings)]
  ifFalse: [ recordingsPresenter clear]! !
!VideoLibraryShell categoriesFor: #onTapeSelected!event handling!private! !


!VideoTape methodsFor!

recordings: aSortedCollection
 recordings := aSortedCollection! !
!VideoTape categoriesFor: #recordings:!public! !


2) There is a bug in the ImageStripper when configured to strip without
removing redundant packages. The method #stripAndSaveNotifying: installs a
RuntimeSessionManager and after that removes development classes. The
RuntimeSessionManager initializes some variables such as resourceLibrary
lazy. The removing of development classes however accesses the
resourceLibrary, so that the RuntimeSessionManager gets initialized at
deploy time instead of runtime yielding to
 to a wrong resource library (e.g. in the case, where the resource library
path is initialized to the image name with an overwritten method
#defaultResLibPath). An example of the call stack:

DonSessionManager>>defaultResLibPath
DonSessionManager(SessionManager)>>defaultResourceLibrary
Icon class(Image class)>>fromId:
BasicInspector class(ClassDescription)>>defaultIcon
BasicInspector class>>icon
FlipperInspector class>>icon
SmalltalkSystemIcon class>>show:description:
FlipperInspector class(SmalltalkToolShell class)>>toolsFolderIcon
FlipperInspector class>>uninitialize
FlipperInspector class(Class)>>uninitializeBeforeRemove
[] in ClassBuilder>>remove
ExceptionHandlerSet(ExceptionHandlerAbstract)>>markAndTry
[] in ExceptionHandlerSet(ExceptionHandlerAbstract)>>try:
BlockClosure>>ifCurtailed:
BlockClosure>>ensure:
ExceptionHandlerSet(ExceptionHandlerAbstract)>>try:
BlockClosure>>onDo:
BlockClosure>>on:do:on:do:
ClassBuilder>>remove
ClassBuilder class>>removeClass:
FlipperInspector class(Class)>>removeFromSystem
[] in ImageStripper>>removeClass:notifying:
ExceptionHandlerSet(ExceptionHandlerAbstract)>>markAndTry
[] in ExceptionHandlerSet(ExceptionHandlerAbstract)>>try:
BlockClosure>>ifCurtailed:
BlockClosure>>ensure:
ExceptionHandlerSet(ExceptionHandlerAbstract)>>try:
BlockClosure>>onDo:
BlockClosure>>on:do:on:do:
ImageStripper>>removeClass:notifying:
[] in ImageStripper>>stripDevelopmentClassesNotifying:

As this is not the only variable initialized lazy, it is not clear whether
other variable might experience the same problem.
Solutions either of:
a) Set the RuntTimeManager as late as possible (then finishedWith has to be
changed)
b) Deinitialize the variables on image startup in method #onStartup:
The bug does only occur, if redundant packages are NOT stripped, because
otherwise the developent classes are already stripped before the
RuntimeSessionManager gets installed.

Our quick bux fix is as follows:


!SessionManager methodsFor!

onStartup: args
 "Initialize the receiver immediately following system startup.
 This is the main system initialization routine, and is responsible
 for starting the windowing system, the process system, etc.
 WARNING: If you break the early startup process (especially before
prestart.st)
 then you will not be able to load your image. It is recommended, therefore,
that
 if changing startup code, you should attempt to run up a new copy of the
image
 just saved BEFORE closing down. Walkbacks will work at just about any
stage,
 but other windows will not open if View>>onStartup has not been run."

 state := 0.
 eventLogHandle := stdioStreams := consoleHandler := nil.
 "BUG: see ImageStripper>>stripAndSaveNotifying:"
 resourceLibrary := nil.

 "Save away the Array of startup arguments, passed in by the VM, for later
use"
 startupArgs := args.
 args class == Array ifTrue: [imagePath := args first] ifFalse: [imagePath
:= args].
 cmdLineFlags := argv := nil.

 [self primaryStartup.
 state := 1. "Mewling and puking in the nurse's arms"

 "Before the secondary startup, lets have an opportunity of getting
 in and fixing and possible problems with the image (only required in
Development?)"
 self preStart.
 self secondaryStartup.
 state := 2. "The whining schoolboy with shining morning face"

 "We are now in a position to claim ownership of the image file, if
relevant."
 self registerRunning.

 "Trigger any user startup processing"
 [self trigger: #sessionStarted] ensure: [self tertiaryStartup].
 state := 3 "The Lover sighing like furnace"]
   ensure:
    ["Always attempt to start the main process (must place into state 5)
   even if earlier startup failed. This may help recover a damaged
   development image if the startup has progressed sufficiently far."
    self forkMain.
    state := 4 "Full of strange oaths and bearded like the pard"].

 "We must terminate the active process to prevent it from continuing from
where it left
 off when the image was saved and running onExit processing, or whatever it
would have
 done next. We also kill it to prevent any invalid termination running."
 Processor activeProcess kill! !
!SessionManager categoriesFor: #onStartup:!event handling!public! !


3) Minor exception problem in the class browser:
a) Open a class browser.
b) Create method Object>>test.
c) Open another class browser.
d) Find the method and open a "Package..." dialog.
e) Switch to the first browser and delete the method.
f) Switch to the second browser and move the method into an another Package
=> Exception and you can't create the method again in this browser.

4) OSVERSIONINFO>>isWinV5 is misleading. The OS version of Win98 is 4 not 5.
There should be method to test for really version 5 or higher (i.e. Windows
2000, Windows XP and Windows Server 2003 ....). #isWinXP for example test
for "at least" Windows XP, whereas all other method are "exact" tests (i.e.
isWin2K exclues Windows XP and highter).

5) New method in SessionManager>>systemWindowsDirectory. Use this instead of
windowsDirectory
in AvatarChat>>characters and RegEdit>>helpTopics, because these crash on a
terminal server. Alternatively
change the implementation of windowsDirectory to that of
systemWindowsDirectory.
systemWindowsDirectory
 "Answer the path of the SHARED Windows system directory on the host
computer
 for the current session. On a single user system this is the same as the
windows directory.
 On a terminal server each user has its own directory where he can store ini
files."

 OSVERSIONINFO current isNT
  ifTrue:
   ["The corresponding API (GetSystemWindowsDirectory) exists only for
Windows 2000 and
   up. For NT4 terminal server edition Microsoft recommends to simply strip
'system32' from
   the system directory. For simplicty we use this for all NT platforms."
   | dir |
   dir := self systemDirectory.
   ((dir copyFrom: dir size - 'system32' size + 1 to: dir size) sameAs:
'system32')
    ifTrue: [^dir copyFrom: 1 to: dir size - ' system32' size]].
 ^self windowsDirectory

6) Bug in DeferredValue in case one changes code in the debugger:
 | a |
 a := 0.
 [ 10/a ] deferredValue value.
In the first exception fix the problem by setting a to 2. Then restart the
block. The old exception is still raised in the calling process.

7) ChoicePresenter has a view resouce "Tree view" which is a good idea but
does not work the the current implementation, because the choicesModel can't
be set by clients of the presenter.

8) TreeView>>addNonVirtualItems:insertStruct: sets #image to new TVITEMs,
but forgets to set  #selectedImage to them.

9) The method IconicListAbstract>>requestDragObjects: uses "self selection"
as a reasonable suggestion for the dragee. But normally you want to drag the
item you picked rather than the item you selected. While there seems to be
no difference between the two in a ListView, in a TreeView you can select
one item, then pick another item, drag and drop it, and get the selected
item from DragDropSession>>dragObjects, but not the picked item! We suggest
the following solution, that as we think fits all cases (in fact the method
ClassSelector>>onDrag: finds this suggestion reasonable):
requestDragObjects: session
"This is where the receiver specifies which object(s) the <DragDropSession>,
session,
is to drag.The objects are added to the session using
DragDropSession>>addDragObject: or DragDropSession>>dragObjects:
Implementation Note: Override to make a reasonable suggestion (the current
selection)."
self hasSelection ifTrue: [session addDragee: session suggestedSource ].
"But let any observers override with their own suggestion"
super requestDragObjects: session

10) The method Menu>>setItem:info: aMENUITEMINFO should use 'self handle'
instead of 'handle' to ensure that the menu is realized. There are already
two positions in the source code which call handle themselves (for this
purpose only) before calling #queryAllFromView: (ListView and
PackageBrowserShell).

11) Misleading comment of File>>isWriteable. The method looks up only the
file attribute, so the file could be effective not writeable, because of
share or ACL restrictions. The comment should mention '... according to the
file attributes'.

12) Aspects should be always be sortable (by sorting after displayString)
and the method #display:on: should also always give a string and never an
exception. Currently every client of these classes has to reimplement the
same functionaliy as in InspectorAbstract class>>textForAspect:  and
PublishedAspectInspector>>getSubAspectsFor:

13) BlockClosure>>isClean is in the category "development" but ProcessCopier
is not and uses isClean.

14) Evaluating ##(1+3) in a Workspace gives an exception instead of 4.

15) When you are using TreeView, sometimes the Dictionary handleObjectMap
gets out of sync with the Windows tree view control. Thus, there may be
times where aTreeView hasSelection is true but aTreeView selection is nil. A
possible reason could be the racing condition issue we discussed this week
here "Possible racing condition when using a modal dialog and a TreeView in
virtual mode and with VirtualTreeModel". Maybe a correct fix is currently
not easily possible, but the behaviour is very annyoing because quite
unpredictable and time consuming to debug (what about a solution in Dolphin
6?).

We use the following workaround:

TreeView>>selectedCount
"Answer the total number of items selected in the receiver."
| selectionHandle |
^((selectionHandle := self selectionByIndex) isNull
or: [(handleObjectMap includesKey: selectionHandle) not]) ifTrue: [0]
ifFalse: [1]

16) CollectionPresenter gives Exception when moving items of a
SortedCollection (move-buttons should be disabled).

17) The up and down-arrows in the report mode of a ListView seems to be in
the DLL DolphinDR005.dll which is 1,4MB in size. In a deployed executable
the report view is missing these arrows. As a workaround we currently
extracted the icons and copied it in our own stub, but be we don't think,
this is the correct solution (because somehow Windows already has these
icons).

18) Debugger problems since Dolphin 5.1. A simple testcase would be:
[#(1 2 3) do: [:each | each = 2 ifTrue: [Error signal: 'Error']]] on: Error
do: [:ex | self halt]
Evaluate this in a workspace and click on Debug. There are the following
problems:
a) I don't see the variable ex
b) The value of each is displayed but the name isn't.

19) An another problem with the debugger is the different behaviour of Step
Into. Debug the following:
[1] value
and "step into". You need three steps to step through it instead of two
steps in D5.0

20) Type ' self halt' in a workspace (without quotes and with the leading
space) and debug it. The selection is displaced by 1 character.

BTW: Is there a rebate for Dolphin 6 upgrade depending on the number of
identified bugs reported? :-))


Best Regards,
straightec GmbH


Reply | Threaded
Open this post in threaded view
|

Re: 20 Bug reports/feature suggestions/fixes for Dolphin 5.1.3

Bill Schwab-2
Maxim,

As a Dolphin use, thanks for organizing these reports, especially the one
about the debugger - I've been bothered by it lately, but hadn't been able
to make time to learn when it started or how to reproduce it.


> 5) New method in SessionManager>>systemWindowsDirectory. Use this instead
of
> windowsDirectory
> in AvatarChat>>characters and RegEdit>>helpTopics, because these crash on
a
> terminal server. Alternatively
> change the implementation of windowsDirectory to that of
> systemWindowsDirectory.
> systemWindowsDirectory
>  "Answer the path of the SHARED Windows system directory on the host
> computer
>  for the current session. On a single user system this is the same as the
> windows directory.
>  On a terminal server each user has its own directory where he can store
ini
> files."

If I'm following you, I'm impressed, grateful for the effort, and must
disagree re where to place it.  The complexity is Microsoft's doing, and so
I suspect (of course I'm not exactly pro-Microsoft) that things will only
get worse over time, and that one would do better to specify a collection of
places to look and one or or more file names that start to suggest the
search has succeeded.  Hopefully, given a CLSID, one could obtain an
interface pointer that would lead to any paths that are required.

In any specific case, one could theoretically query the cached MSI files.
However, I would not like to see Dolphin to become dependent on them.  The
Windows Installer is very poorly designed (dangerously so, IMHO).

In the end, I'll be happy as long as I have a clean way to query for the
windows and system directories.  Any sophistication one top of those basic
queries will be appreciated if needed.

Have a good one,

Bill

--
Wilhelm K. Schwab, Ph.D.
[hidden email]


Reply | Threaded
Open this post in threaded view
|

Re: 20 Bug reports/feature suggestions/fixes for Dolphin 5.1.3

Ian Bartholomew-18
In reply to this post by Maxim Fridental
Maxim,

> 14) Evaluating ##(1+3) in a Workspace gives an exception instead of 4.

That one surprised me so I tried it in my 5.1.3.  It works OK for me -
no exception?

--
Ian


Reply | Threaded
Open this post in threaded view
|

Re: 20 Bug reports/feature suggestions/fixes for Dolphin 5.1.3

Maxim Fridental
Ian,

> > 14) Evaluating ##(1+3) in a Workspace gives an exception instead of 4.
> That one surprised me so I tried it in my 5.1.3.  It works OK for me -
> no exception?
Yes, you are right. It's SSWSmalltalkWorkspace>>compileRange:ifFail:debug:
who throws the exception, so it's really a bug of Tools+. I've overlooked
it. Thank you.

Best Regards,
Maxim Fridental


Reply | Threaded
Open this post in threaded view
|

Re: 20 Bug reports/feature suggestions/fixes for Dolphin 5.1.3

Andy Bower-3
In reply to this post by Maxim Fridental
Maxim,

Thanks for the work you've put into these reports. It may take a few
days to assimilate what you have given to us but we'll report any
comments via this thread.

--
Andy Bower
Dolphin Support
www.object-arts.com


Reply | Threaded
Open this post in threaded view
|

Re: 20 Bug reports/feature suggestions/fixes for Dolphin 5.1.3

Blair McGlashan
In reply to this post by Maxim Fridental
Maxim

You wrote in message news:bql7h1$22f7p4$[hidden email]...
> Blair,
>
> thank you very much for your support. ...

And thank you for your feedback.

>...We would like to report some other
> bugs we've found. Our development environment is Dolphin Pro 5.1.3 ...

Erm, PL3 hasn't been formally released yet, so I assume you mean 5.1.2?

Unfortunately these reports mostly too late for patching in PL3 (even where
feasible) as its contents have been frozen for final testing. Changes
limited to the modification of comments or method categories can be
included, but code modifications are out.

>... under
> Windows 2000 Terminal Server. Note, that we have already posted some
> problems in the newsgroup, but received no answer.
>
>
> 1) Problem with beSorted: (ListView replaces the old collection in the
> ListModel with a new sorted collection). Recorded by Andy Bower as defect
> #1134. One possible solution is to extend the ListModel, so that is also
get
> its list from a ValueModel.

Thanks for the suggestion, I'll add it as a comment to the defect report. We
aren't intending to address this in a patch because we think it may need a
breaking change.

> 2) There is a bug in the ImageStripper when configured to strip without
> removing redundant packages. The method #stripAndSaveNotifying: installs a
> RuntimeSessionManager and after that removes development classes. The
> RuntimeSessionManager initializes some variables such as resourceLibrary
> lazy. The removing of development classes however accesses the
> resourceLibrary, so that the RuntimeSessionManager gets initialized at
> deploy time instead of runtime yielding to
>  to a wrong resource library (e.g. in the case, where the resource library
> path is initialized to the image name with an overwritten method
> #defaultResLibPath). ...

Recorded as #1420.

As a workaround I would suggest overriding #primaryStartup in your
RuntimeSessionManager as follows:

primaryStartup
    resourceLibrary := nil.
    super primaryStartup

Another solution (which doesn't really address the basic issue, but which
should be changed anyway) is to modify the class #uninitialize methods that
use #removeSystemFolderIcon: to use #systemFolderIconNamed:, since this
avoid the unecessary instantiation of a SystemFolderIcon instance in order
to remove the registration.

> 3) Minor exception problem in the class browser:
> a) Open a class browser.
> b) Create method Object>>test.
> c) Open another class browser.
> d) Find the method and open a "Package..." dialog.
> e) Switch to the first browser and delete the method.
> f) Switch to the second browser and move the method into an another
Package
> => Exception and you can't create the method again in this browser.
>

OK, #1421.

BTW I think this one is verging on the border between a genuine issue and a
"so don't do that". In Smalltalk it is possible to make all sorts of
dangerous changes that will break the tools or image. It isn't feasible to
guard against or handle all these cases without compromising the essential
nature of the environment.

> 4) OSVERSIONINFO>>isWinV5 is misleading. The OS version of Win98 is 4 not
5.

True, it is misleading now. At the time the method was written it mean't
whether or not the version 5 Windows UI was supported. This was first
introduced in Win98 (which, as you say, still used the same major version
number as Win95), and Microsoft used this terminology themselves in some
places in the SDK, for example in the naming of the BITMAPV5HEADER
structure.  I will make sure that this is clarified in the method comment
for a future release.

>....
> There should be method to test for really version 5 or higher (i.e.
Windows
> 2000, Windows XP and Windows Server 2003 ....). #isWinXP for example test
> for "at least" Windows XP, whereas all other method are "exact" tests
(i.e.
> isWin2K exclues Windows XP and highter).

True, there should be a more rigorous pattern applied to the naming of these
methods. They have been added to support requirements to identify OS version
information as the need has arisen, rather than with any particular scheme
in mind.

> 5) New method in SessionManager>>systemWindowsDirectory. Use this instead
of
> windowsDirectory
> in AvatarChat>>characters and RegEdit>>helpTopics, because these crash on
a
> terminal server.  change the implementation of windowsDirectory to that of
> systemWindowsDirectory.

I'll add that as an enhancement #1422. We'll probably replace
#windowsDirectory with a shared implementation since we don't think there is
any need to write files to a user specific Windows directory any more.

> 6) Bug in DeferredValue in case one changes code in the debugger:
>  | a |
>  a := 0.
>  [ 10/a ] deferredValue value.
> In the first exception fix the problem by setting a to 2. Then restart the
> block. The old exception is still raised in the calling process.

That is by design. Restarting a block does not in any way guarantee that any
side effects of previous execution of the block will be undone. In fact in
D5 the block temps are not even reinitialized to nil because they cannot be
distinguished from the method temporaries and the block's own arguments -
this is fixed in D6 of course as a side benefit of the full-closure
implementation.

> 7) ChoicePresenter has a view resouce "Tree view" which is a good idea but
> does not work the the current implementation, because the choicesModel
can't
> be set by clients of the presenter.

Recorded as #1423.

> 8) TreeView>>addNonVirtualItems:insertStruct: sets #image to new TVITEMs,
> but forgets to set  #selectedImage to them.

Recorded as #1424.

> 9) The method IconicListAbstract>>requestDragObjects: uses "self
selection"
> as a reasonable suggestion for the dragee. But normally you want to drag
the
> item you picked rather than the item you selected. While there seems to be
> no difference between the two in a ListView, in a TreeView you can select
> one item, then pick another item, drag and drop it, and get the selected
> item from DragDropSession>>dragObjects, but not the picked item! We
suggest
> the following solution, that as we think fits all cases (in fact the
method
> ClassSelector>>onDrag: finds this suggestion reasonable):
> requestDragObjects: session
> "This is where the receiver specifies which object(s) the
<DragDropSession>,
> session,
> is to drag.The objects are added to the session using
> DragDropSession>>addDragObject: or DragDropSession>>dragObjects:
> Implementation Note: Override to make a reasonable suggestion (the current
> selection)."
> self hasSelection ifTrue: [session addDragee: session suggestedSource ].
> "But let any observers override with their own suggestion"
> super requestDragObjects: session

Yes, it should be the default behaviour. Enhancement #1425.

> 10) The method Menu>>setItem:info: aMENUITEMINFO should use 'self handle'
> instead of 'handle' to ensure that the menu is realized. There are already
> two positions in the source code which call handle themselves (for this
> purpose only) before calling #queryAllFromView: (ListView and
> PackageBrowserShell).

Well spotted, #1426.

> 11) Misleading comment of File>>isWriteable. The method looks up only the
> file attribute, so the file could be effective not writeable, because of
> share or ACL restrictions. The comment should mention '... according to
the
> file attributes'.

I presume you mean the class method #isWriteable: rather than the instance
method #isWriteable (the latter tests against the mode used to open the
file, and a FileException should have been raised if that mode did not pass
security checks)?

Anyway I disagree that it is misleading. The comment employs the usual
meaning of the phrase 'writeable file' in this context.

> 12) Aspects should be always be sortable (by sorting after displayString)
> and the method #display:on: should also always give a string and never an
> exception. Currently every client of these classes has to reimplement the
> same functionaliy as in InspectorAbstract class>>textForAspect:  and
> PublishedAspectInspector>>getSubAspectsFor:

Possibly, but apart from the fact that these classes are not intended for
application use (and have in fact been refactored in D6, InspectorAbstract
class>>textForAspect: no longer exists), sorting by the display string might
not yield the desired sort order.

>
> 13) BlockClosure>>isClean is in the category "development" but
ProcessCopier
> is not and uses isClean.

Thanks, #1427.

>
> 14) Evaluating ##(1+3) in a Workspace gives an exception instead of 4.

Really? I can't reproduce that in 5.1.2.

>
> 15) When you are using TreeView, sometimes the Dictionary handleObjectMap
> gets out of sync with the Windows tree view control. Thus, there may be
> times where aTreeView hasSelection is true but aTreeView selection is nil.
A
> possible reason could be the racing condition issue we discussed this week
> here "Possible racing condition when using a modal dialog and a TreeView
in
> virtual mode and with VirtualTreeModel". Maybe a correct fix is currently
> not easily possible, but the behaviour is very annyoing because quite
> unpredictable and time consuming to debug (what about a solution in
Dolphin
> 6?).

We have never experienced this, and have no other reports either. It is
quite possible that it is due to thread synchronisation issues causing
multiple processes to be accessing the handle object map, e.g. one process
is repainting while another is updating the map. The manner in which dialogs
are implemented can cause this problem if one is not aware of it, but this
can be solved as discussed in the thread you mention. We don't think that
adding workarounds to the TreeView is the correct thing to do because this
just treats the symptom and hides any underlying issue.

> 16) CollectionPresenter gives Exception when moving items of a
> SortedCollection (move-buttons should be disabled).

Yes, #1428.

>
> 17) The up and down-arrows in the report mode of a ListView seems to be in
> the DLL DolphinDR005.dll which is 1,4MB in size. In a deployed executable
> the report view is missing these arrows. As a workaround we currently
> extracted the icons and copied it in our own stub, but be we don't think,
> this is the correct solution (because somehow Windows already has these
> icons).

I think this may be one of those areas where there is a variation between
different versions of Windows. Certainly with the version 6 common controls
(Windows XP) this can be done simply by setting a flag, but this is not
available on earlier versions. I don't know the precise reason the icons
were copied, but I suspect it may be because they were not published as
existing at a consistent location. However the icons should be in the
standard stub already, since they are used by a common MVP view and are not
specific to the development system, so #1429.

> 18) Debugger problems since Dolphin 5.1. A simple testcase would be:
> [#(1 2 3) do: [:each | each = 2 ifTrue: [Error signal: 'Error']]] on:
Error
> do: [:ex | self halt]
> Evaluate this in a workspace and click on Debug. There are the following
> problems:
> a) I don't see the variable ex

This is a known issue with the D5.1 compiler that shows up in occassional
circumstances (it has to do with the text maps it builds for the debugger).

> b) The value of each is displayed but the name isn't.

This is by design. 'each' is logically out of scope at that point, so the
Debugger does not show its name. The value is still present because of the
Smalltalk-80 style block implementation in D5 meaning that it is physically
still in scope. In D6 neither name nor value is available because the
variable has both logically and physically gone out of scope.

> 19) An another problem with the debugger is the different behaviour of
Step
> Into. Debug the following:
> [1] value
> and "step into". You need three steps to step through it instead of two
> steps in D5.0

I'll look into that (#1431). I think it may have come about because of
another change to ensure that one always stops on first entry to a block -
something which did not always happen in some obscure cases in earlier
versions. The debugger should always continue to the first breakpoint in the
block, which it does in D6 although there the debugger has significant
modifications because of the new blocks.

>
> 20) Type ' self halt' in a workspace (without quotes and with the leading
> space) and debug it. The selection is displaced by 1 character.

This is a bug in either the RichEdit control, or (more likely in this case)
the RTF generated by the compiler. I'll record it (for your score :-))but it
is unlikely to be addressed for D5 since we don't intend to ship another
compiler DLL before D6, unless any serious issues are found. The D6 compiler
no longer generates RTF, and the RichEdit control is no longer used for
source editing.

>
> BTW: Is there a rebate for Dolphin 6 upgrade depending on the number of
> identified bugs reported? :-))

Perhaps, but I'm afraid you only get one point per posting. We welcome
feedback, but prefer it in smaller mouthfulls :-).

Regards

Blair


Reply | Threaded
Open this post in threaded view
|

Re: 20 Bug reports/feature suggestions/fixes for Dolphin 5.1.3 beta

Carsten Haerle
Blair,

Thanks for your quick answer. Some comments below:

> >...We would like to report some other
> > bugs we've found. Our development environment is Dolphin Pro 5.1.3 ...
>
> Erm, PL3 hasn't been formally released yet, so I assume you mean 5.1.2?

It should read "5.1.3 beta", because we already integrated this with our
code version and postest all remaining open problems.

> > 1) Problem with beSorted: (ListView replaces the old collection in the
> > ListModel with a new sorted collection). Recorded by Andy Bower as
defect
> > #1134. One possible solution is to extend the ListModel, so that is also
> get
> > its list from a ValueModel.
>
> Thanks for the suggestion, I'll add it as a comment to the defect report.
We
> aren't intending to address this in a patch because we think it may need a
> breaking change.

Maybe there is better solutions, but our solutions is NOT breaking. Maybe it
were also better to sublcasse ListModel with and our changes into this
subclass. Another solution we thought about is to always sort in place,
because the other operations like remove, add and swap are also in place.
But then you have a problem with Collections which are not resortable
(SortedCollectio, ADOCollection, and so on). Maybe you find an even better
solution.

> > 3) Minor exception problem in the class browser:
> > a) Open a class browser.
> > b) Create method Object>>test.
> > c) Open another class browser.
> > d) Find the method and open a "Package..." dialog.
> > e) Switch to the first browser and delete the method.
> > f) Switch to the second browser and move the method into an another
> Package
> > => Exception and you can't create the method again in this browser.
> >
>
> OK, #1421.
>
> BTW I think this one is verging on the border between a genuine issue and
a
> "so don't do that". In Smalltalk it is possible to make all sorts of
> dangerous changes that will break the tools or image. It isn't feasible to
> guard against or handle all these cases without compromising the essential
> nature of the environment.

BTW: We discovered this problem because we have a similiar problem in our
application. We want to have a truely event driven GUI, which
instantaneously displays the current state of the object model without
saying "Refresh" (which we currently have to), the Dolphin development
system is much like what we also want, so we just check "how did they solve
a particular problem", and one of the problems was, having a model dialog
open and chaning underlying model from a different source (this would be
done through a background process in our application). Unfortunately you
also didn't solve this problem, which would have saved us thinking about the
problem ourselves.

> > 5) New method in SessionManager>>systemWindowsDirectory. Use this
instead
> of
> > windowsDirectory
> > in AvatarChat>>characters and RegEdit>>helpTopics, because these crash
on
> a
> > terminal server.  change the implementation of windowsDirectory to that
of
> > systemWindowsDirectory.
>
> I'll add that as an enhancement #1422. We'll probably replace
> #windowsDirectory with a shared implementation since we don't think there
is
> any need to write files to a user specific Windows directory any more.

Yes this was my first idea also, but then I then I thought, that the correct
solutions would be to keep conform with the Microsoft naming. But maybe it
is even better to violate this and simply use #windowsDirectory, because
most applications would do the right thing, and only application with
INI-files placed in the system directory (e.g. win31 style applications)
would have a problem. It would also not introduce a new naming and would
require changing client code. So use #windowsDirecory.

> > 11) Misleading comment of File>>isWriteable. The method looks up only
the
> > file attribute, so the file could be effective not writeable, because of
> > share or ACL restrictions. The comment should mention '... according to
> the
> > file attributes'.
>
> I presume you mean the class method #isWriteable: rather than the instance
> method #isWriteable (the latter tests against the mode used to open the
> file, and a FileException should have been raised if that mode did not
pass
> security checks)?
>
> Anyway I disagree that it is misleading. The comment employs the usual
> meaning of the phrase 'writeable file' in this context.

It is a little misleadingif you read the comment, because most people know
the windows flag "readonly" and if they see the class side method #writable:
they may thing that a result of "true" garanties that the file can be
written, which is not the case, because ACLs or sharing restrictions could
still prevent this. The method comment is not clear about this, and we
already had this bug, because assumed that when he gets true from
isWriteable:  that he could write into this file.

> > 12) Aspects should be always be sortable (by sorting after
displayString)
> > and the method #display:on: should also always give a string and never
an
> > exception. Currently every client of these classes has to reimplement
the
> > same functionaliy as in InspectorAbstract class>>textForAspect:  and
> > PublishedAspectInspector>>getSubAspectsFor:
>
> Possibly, but apart from the fact that these classes are not intended for
> application use (and have in fact been refactored in D6, InspectorAbstract
> class>>textForAspect: no longer exists), sorting by the display string
might
> not yield the desired sort order.

I don't know what will be there in D6, but two remarks:
a) I had the above problem because I extended the development system and
needed just the properties of some objects in a decent order and ended up
copying source code. I agree #displayString sorting is not correct, maybe
sorting after class and afterwards after value would be better, but anyway
they should be always sortable.
b) Why is this only in the development system. Many applications do only a
little more than editing standard published attributes and relations. I
would be a great thing if this kind of applications could be written easier
with Dolphin. Dolphin Smalltalk itsself uses this trick in some places as in
the view composer or in the system options. The presentation looks almost
like a hand craftet user interface specially for the "options" and views but
in reality it is just a generic inspect. Now take this idea one stop further
and allow a little more customization of the view and use this in normal
application. In our application we have seveal "properties" dialogs which
just allow the display and editing of a subset of attribute (published
attributes) of our objects. We did all the dialogs by hand, but this is time
consuming, and the generic inspector framework shows the way to go in my
opinion.

> > 17) The up and down-arrows in the report mode of a ListView seems to be
in
> > the DLL DolphinDR005.dll which is 1,4MB in size. In a deployed
executable
> > the report view is missing these arrows. As a workaround we currently
> > extracted the icons and copied it in our own stub, but be we don't
think,
> > this is the correct solution (because somehow Windows already has these
> > icons).
>
> I think this may be one of those areas where there is a variation between
> different versions of Windows. Certainly with the version 6 common
controls
> (Windows XP) this can be done simply by setting a flag, but this is not
> available on earlier versions. I don't know the precise reason the icons
> were copied, but I suspect it may be because they were not published as
> existing at a consistent location. However the icons should be in the
> standard stub already, since they are used by a common MVP view and are
not
> specific to the development system, so #1429.

How does the explorer in windows 2000 display these icons. And there are
quote a lot of other applications which use the reprort view, and I don't
think they all copied the icond into there own resource library, did they?

> > BTW: Is there a rebate for Dolphin 6 upgrade depending on the number of
> > identified bugs reported? :-))
>
> Perhaps, but I'm afraid you only get one point per posting. We welcome
> feedback, but prefer it in smaller mouthfulls :-).

Ok. Let's see what we can do in the future :-)

Regards

Carsten


Reply | Threaded
Open this post in threaded view
|

Re: 20 Bug reports/feature suggestions/fixes for Dolphin 5.1.3 beta

Blair McGlashan
Carsten

Re: message news:bqom1l$jth$04$[hidden email]...
>...
> > > 1) Problem with beSorted: (ListView replaces the old collection in the
> > > ListModel with a new sorted collection). Recorded by Andy Bower as
> defect
> > > #1134. One possible solution is to extend the ListModel, so that is
also
> > get
> > > its list from a ValueModel.
> >
> > Thanks for the suggestion, I'll add it as a comment to the defect
report.
> We
> > aren't intending to address this in a patch because we think it may need
a
> > breaking change.
>
> Maybe there is better solutions, but our solutions is NOT breaking. Maybe
it
> were also better to sublcasse ListModel with and our changes into this
> subclass. Another solution we thought about is to always sort in place,
> because the other operations like remove, add and swap are also in place.
> But then you have a problem with Collections which are not resortable
> (SortedCollectio, ADOCollection, and so on). Maybe you find an even better
> solution.

I wasn't suggesting that your solution was a breaking change - we haven't
had time to examine it yet. I meant that we thought it would need a breaking
change to solve it in an elegant way for the general case based on our
previous analysis.

>
> > > 3) Minor exception problem in the class browser:
> >...
> > BTW I think this one is verging on the border between a genuine issue
and
> a
> > "so don't do that". In Smalltalk it is possible to make all sorts of
> > dangerous changes that will break the tools or image. It isn't feasible
to
> > guard against or handle all these cases without compromising the
essential
> > nature of the environment.
>
> BTW: We discovered this problem because we have a similiar problem in our
> application. We want to have a truely event driven GUI, which
> instantaneously displays the current state of the object model without
> saying "Refresh" (which we currently have to), the Dolphin development
> system is much like what we also want, so we just check "how did they
solve
> a particular problem", and one of the problems was, having a model dialog
> open and chaning underlying model from a different source (this would be
> done through a background process in our application). Unfortunately you
> also didn't solve this problem, which would have saved us thinking about
the
> problem ourselves.

Well there is no one general solution to the problem of an end user being
able to delete an object from one window that one is modifying through a
dialog in another. I'm sure you've thought of these already, but in an
application you could:
1) Close the dialog - this would be a rather "rude" UI.
2) Disable the OK button in the dialog (i.e. make the continued existence of
the object being updated part of the validation)
3) Handle the issue by checking that it is still valid to proceed when the
dialog is closed, probably giving some error report (other than the walkback
one gets in the situation you reported :-)).
4) "Lock" the object when it is being edited by a dialog, and disallow its
deletion or update elsewhere
5) Make the dialog completely modal and therefore prevent the user from
carrying out mutually exclusive modifications.

Which of these to use (or any other approach) will depend on the case,
though obviously (1) is a rather poor choice. I don't like (3) much either,
since the feedback is not immediate and possibly delayed until after the
user has spent some time doing more work.  One sees this quite a bit in
applications that use optimistic locking in that they will allow the user to
spend time and effort modify something that then gets rejected because of a
change elsewhere. This might be a useful approach for building scaleable
applications, but doesn't necessarily provide the best usability.

Anyway I was just pointing out that in a development system (especially one
like Smalltalk) there is a limit to the tightness of the validation that one
can provide without compromising the flexibility of the system, and
certainly the easy option of simply disabling everything else (i.e. 5 above)
and making the dialog completely modal is very inconvenient. Actually I find
it very inconvenient in many applications too, so I quite understand why you
would want to minimise modality. However if you do so then your job will
certainly be a lot harder, especially in an end-user application where
strict validation is more of a requirement than it is in a developer tool.

>
> > > 5) New method in SessionManager>>systemWindowsDirectory.
> >....
> > I'll add that as an enhancement #1422. We'll probably replace
> > #windowsDirectory with a shared implementation since we don't think
there
> is
> > any need to write files to a user specific Windows directory any more.
>
> Yes this was my first idea also, but then I then I thought, that the
correct
> solutions would be to keep conform with the Microsoft naming. But maybe it
> is even better to violate this and simply use #windowsDirectory, because
> most applications would do the right thing, and only application with
> INI-files placed in the system directory (e.g. win31 style applications)
> would have a problem. It would also not introduce a new naming and would
> require changing client code. So use #windowsDirecory.

My thinking exactly.

>
> > > 11) Misleading comment of File>>isWriteable.
>...
> It is a little misleadingif you read the comment, because most people know
> the windows flag "readonly" and if they see the class side method
#writable:
> they may thing that a result of "true" garanties that the file can be
> written, which is not the case, because ACLs or sharing restrictions could
> still prevent this. The method comment is not clear about this, and we
> already had this bug, because assumed that when he gets true from
> isWriteable:  that he could write into this file.

OK, I will modify the comment (#1433).

>
> > > 12) Aspects should be always be sortable (by sorting after
> displayString)...
> ...
> I don't know what will be there in D6, but two remarks:
> a) I had the above problem because I extended the development system and
> needed just the properties of some objects in a decent order and ended up
> copying source code. I agree #displayString sorting is not correct, maybe
> sorting after class and afterwards after value would be better, but anyway
> they should be always sortable.

Well the Aspects are always sortable in theory. The DNU handling is to cope
with the case where the keys of KeyedAspects are not themselves orderable
with #<=. Here's an example:

(Dictionary new at: Object new put: 1; at: Object new put: 2; yourself)
publishedAspects asSortedCollection

It is possible that this could be handled more elegantly I agree.

> b) Why is this only in the development system. Many applications do only a
> little more than editing standard published attributes and relations. I
> would be a great thing if this kind of applications could be written
easier
> with Dolphin. Dolphin Smalltalk itsself uses this trick in some places as
in
> the view composer or in the system options. The presentation looks almost
> like a hand craftet user interface specially for the "options" and views
but
> in reality it is just a generic inspect. ...

I can quite understand why you would want to make use of the facility. The
reasons we regard it as 'development' are:
1) The design doesn't provide enough validation for use in an application.
2) It would need to be refactored to remove other development system
dependencies.
3) We wanted to maintain the freedom to change the design and implementation
of the published aspect system and inspector.

Far and away the most important of these is (1). It is also the most
difficult to address because it was not considered in the design. If you
wanted to use the Aspect stuff in an application, we wouldn't object, but it
would need quite a lot of work IMO.

> > > 17) The up and down-arrows in the report mode of a ListView seems to
be
> in
> > > the DLL DolphinDR005.dll which is 1,4MB in size. In a deployed
> executable
> > > the report view is missing these arrows.
>...

> How does the explorer in windows 2000 display these icons.

I suspect in the same way that we do - i.e. by pulling icons from resources.
It might be that they are actually stored as simple bitmaps though, as I
can't remember where we got them from.

>...And there are
> quote a lot of other applications which use the reprort view, and I don't
> think they all copied the icond into there own resource library, did they?

AFAIK the icons are not documented as public resources in the Microsoft
Platform SDK. If you or anyone else knows otherwise, then I'd be pleased to
hear about it

>
> > > BTW: Is there a rebate for Dolphin 6 upgrade depending on the number
of
> > > identified bugs reported? :-))
> >
> > Perhaps, but I'm afraid you only get one point per posting. We welcome
> > feedback, but prefer it in smaller mouthfulls :-).
>
> Ok. Let's see what we can do in the future :-)

Thanks, that would be appreciated.

Blair