#14337: wxRendererXP::DrawChoice and presumably other methods as well broken

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/14337>

#14337: wxRendererXP::DrawChoice and presumably other methods as well broken
--------------------------------------------------------------------+-------
Reporter: mmacleod | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: wxMSW | Version: 2.9-svn
Keywords: DrawChoice wxRendererNative wxRendererXP wxRendererMSW | Blockedby:
Patch: 1 | Blocking:
--------------------------------------------------------------------+-------
wxRendererXP::DrawChoice does not work correctly, it results in
wxRendererMSW incorrectly handling the pulldown arrow instead of
wxRendererXP doing so.
This seems to be because for some reason proper inheritance despite all
functions being virtual, but instead some broken attempt at reimplementing
inheritance seems to be used.
The attached patch fixes wxRendererXP to use normal/proper inheritance and
also makes wxRendererXP::DrawChoice behave correctly as a result
(Presumably there are other methods that may have been broken in a similar
way which will now be correct)


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14337>

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/14337#comment:1>

#14337: wxRendererXP::DrawChoice and presumably other methods as well broken
--------------------------------------------------------------------+-------
Reporter: mmacleod | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: wxMSW | Version: 2.9-svn
Keywords: DrawChoice wxRendererNative wxRendererXP wxRendererMSW | Blockedby:
Patch: 1 | Blocking:
--------------------------------------------------------------------+-------

Comment(by mmacleod):

Sorry the above should read "because for some reason proper inheritance is
not used"


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14337#comment:1>

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/14337#comment:2>

#14337: wxRendererXP::DrawChoice() wrongly delegates to wxRendererMSW

--------------------------------------------------------------------+-------
Reporter: mmacleod | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: wxMSW | Version: 2.9-svn
Keywords: DrawChoice wxRendererNative wxRendererXP wxRendererMSW | Blockedby:
Patch: 1 | Blocking:
--------------------------------------------------------------------+-------

Comment(by vadz):

Composition is used for renderer classes because in general the renderer
to use is determined during run-, not compile-time. It's true that in this
particular case we could do what your patch does but I think it would be
simpler and better to just move both `DrawComboBox()` and `DrawChoice()`
down to `wxRendererMSWBase`, this is the class used for methods common to
both classic and XP renderers. We'd also need to declare pure virtual
`DrawComboBoxDropButton()` and `DrawTextCtrl()` in it (the latter is
already a mix of classic and XP code so splitting it into 2 parts would be
really better anyhow). Is there any problem with doing this?

I'd prefer to keep things as they are to keep these classes uncoupled,
e.g. to be able to remove `wxRendererMSW` easily later.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14337#comment:2>

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/14337#comment:3>

#14337: wxRendererXP::DrawChoice() wrongly delegates to wxRendererMSW

--------------------------------------------------------------------+-------
Reporter: mmacleod | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: wxMSW | Version: 2.9-svn
Keywords: DrawChoice wxRendererNative wxRendererXP wxRendererMSW | Blockedby:
Patch: 1 | Blocking:
--------------------------------------------------------------------+-------

Comment(by mmacleod):

No complaints from this side on how it is fixed as long as there is a fix
of some kind.

I do wonder if doing composition in this way is not prone to cause bugs
like this though, as it is looks painfully easy to forget that the two are
not interacting with one another in the way one would expect - however I
know nothing about this code, and don't maintain it so I can't really
judge or say what would be best.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14337#comment:3>

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/14337#comment:4>

#14337: wxRendererXP::DrawChoice() wrongly delegates to wxRendererMSW

--------------------------------------------------------------------+-------
Reporter: mmacleod | Owner:
Type: defect | Status: confirmed

Priority: normal | Milestone:
Component: wxMSW | Version: 2.9-svn
Keywords: DrawChoice wxRendererNative wxRendererXP wxRendererMSW | Blockedby:
Patch: 1 | Blocking:
--------------------------------------------------------------------+-------

Changes (by vadz):

* status: new => confirmed


Comment:

If you could update your patch to do it like this, it'd be great.
Otherwise, would you have some test code, e.g. a simple patch to the
renderer sample or something like this? TIA!


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14337#comment:4>

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/14337#comment:5>

#14337: wxRendererXP::DrawChoice() wrongly delegates to wxRendererMSW

--------------------------------------------------------------------+-------
Reporter: mmacleod | Owner:
Type: defect | Status: confirmed

Priority: normal | Milestone:
Component: wxMSW | Version: 2.9-svn
Keywords: DrawChoice wxRendererNative wxRendererXP wxRendererMSW | Blockedby:
Patch: 1 | Blocking:
--------------------------------------------------------------------+-------

Comment(by mmacleod):

Update patch attached.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14337#comment:5>

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/14337#comment:6>

#14337: wxRendererXP::DrawChoice() wrongly delegates to wxRendererMSW

-----------------------+----------------------------------------------------
Reporter: mmacleod | Owner:
Type: defect | Status: closed

Priority: normal | Milestone:
Component: wxMSW | Version: 2.9-svn

Resolution: fixed | Keywords: DrawChoice wxRendererNative wxRendererXP wxRendererMSW
Blockedby: | Patch: 1
Blocking: |
-----------------------+----------------------------------------------------
Changes (by VZ):

* status: confirmed => closed
* resolution: => fixed


Comment:

(In [71569]) Fix DrawChoice() behaviour in wxRendererXP.

The implementation of wxRendererXP::DrawChoice() inadvertently used
wxRendererMSW::DrawComboBoxDropButton() and so drew the button in the
classic
and not themed style.

Fix this by defining DrawChoice() in the base wxRendererMSWBase class and
reusing it in wxRendererXP via inheritance and not composition.

Closes #14337.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/14337#comment:6>