rsargent wrote:The fundamental problem is the "assumption" that every owner of a shell has its own owner.
Not exactly. The assumption (which is perfectly valid given the intended usage of the
owner field in
OSWidget) is that every
OSWidget will either have no owner (nil) or that its owner with be a
CwWidget instance (which will itself respond to
#owner).
You can actually test this assumption in your own image with the following Transcript doit (open any number of other browsers or other windows that you like before running this)...
- Code: Select all
OSWidget withAllSubclasses do: [:sc |
sc allInstances do: [:osw |
osw owner notNil ifTrue: [
(osw owner isKindOf: CwWidget) ifFalse: [
Transcript cr; show: 'not a CwWidget'.
osw owner inspect]]]]
You shouldn't see any instances of non nill owners that are not
CwWidgets.
The problem here is actually with the
CwTextPrompter>>#prompt method which uses a clever (but nasty) hack to set the owner to a block which will later respond to the
#value: message. Does it work? Yes. Could it have been written differently so as not to violate the intended usage of the
owner field? Most certainly.
rsargent wrote:What you are trying to do is determine whether the owner represents a W.B. instance.
To be more specific, I want to know whether the owner's owner (if it has one) is either an EtWindow instance or a WbApplication instance. If you were to run that code snippet against every OSWidget instance in your image, it would not fail.
rsargent wrote:So ask the owner that question and implement the answer in Object as ^false, and implement the answer in the expected owner classes as a redirection to the owner's owner.
That sounds like an even more complex solution that would require even more code.
rsargent wrote:FWIW, the beginning code in OSShell>>#wmInitmenupopup:with: would have failed any code review I would have provided.
Only because you are not aware of the context or the constraints upon the solution. That snippet of code that you don't like is a hack intended to work around a base image bug introduced in VAST 5.01(?) which broke the use of menu accelerator keys in various windows. One of the cardinal rules of "surgical hacking" like this is to keep it as isolated as possible and introduce as little additional scaffolding (other methods and class extensions) as possible. As it is, that hack was isolated to one and only one method, and it has worked very well for many, many years.
In your example, you have found about the edgiest edge condition I have ever seen (which does, in fact, cause that code to fail). The simplest solution is not to rewrite the original hack to introduce additional methods at several places in the hierarchy, but to implement the one simple method I posted earlier (or even make it more specific and add it to
Block rather than
Object). If there were actually other places where we needed to know whether the owner's owner was one of those types, it might be worth the extra overhead.
Of course, now that we control all of this code, maybe the hack can be removed and the original base image code modified instead. Doing that, however, raises other issues about making changes to long standing base image code. It's usually better to fix a very isolated bug like this one than to make wholesale changes that could have unintended consequences elsewhere (especially in user code).
BTW, the original hack could be slightly improved as follows since the intermediate
#isNil test is not needed (every object should respond to
#class)...
- Code: Select all
...
(self owner isNil
or: [self owner owner class isWbApplicationClass not])
ifTrue: [^super wmInitmenupopup: wParam with: lParam].
...
