Revision #15307 fixes bug #1740146, in which changes in the document-properties dialog are written to the wrong document when multiple documents are open.
In registered-widget.cpp, we used to have
if (!repr) repr = SP_OBJECT_REPR (sp_desktop_namedview(dt));
but this won't update "repr" when the focus has changed to another document. Just removing the if-statement did the trick, but this is also needed for the other controls. We have two other options though: - make "repr" a local variable instead of a class-variable. Why should it be a class-variable after all? - have a mechanism to notify the class of changes in the focus and update "repr" accordingly, but I don't see a reason for doing it like this.
I'm not very familiar with the GUI stuff yet, so some pointers in the right direction would be appreciated.
Diederik
On 7/6/07, Diederik van Lierop <mail@...1689...> wrote:
if (!repr) repr = SP_OBJECT_REPR (sp_desktop_namedview(dt));
but this won't update "repr" when the focus has changed to another document. Just removing the if-statement did the trick, but this is also needed for the other controls.
So, does dt change when you switch windows? If so, then of course, it's perfectly OK to calculare repr from dt unconditionally.
We have two other options though:
- make "repr" a local variable instead of a class-variable. Why should
it be a class-variable after all?
No idea. Indeed it makes sense to only store and update the desktop and get everything else from it.
On 7/6/07, Diederik van Lierop <mail@...1689...> wrote:
if (!repr) repr = SP_OBJECT_REPR (sp_desktop_namedview(dt));
By the way, svn history shows this code was added relatively recently by Johan Engelen. Before his change, repr was a local variable assigned unconditionally. In such cases it may make sense to ask the author first.
Diederik van Lierop wrote:
Revision #15307 fixes bug #1740146, in which changes in the document-properties dialog are written to the wrong document when multiple documents are open.
In registered-widget.cpp, we used to have
if (!repr) repr = SP_OBJECT_REPR (sp_desktop_namedview(dt));
but this won't update "repr" when the focus has changed to another document. Just removing the if-statement did the trick, but this is also needed for the other controls. We have two other options though:
- make "repr" a local variable instead of a class-variable.
Why should it be a class-variable after all?
- have a mechanism to notify the class of changes in the
focus and update "repr" accordingly, but I don't see a reason for doing it like this.
repr is a class variable, because these widgets are also used for things that are written elsewhere in the xml tree (for example grid variables). repr specifies where that is. When it is null, it should use the default location, which is 'SP_OBJECT_REPR (sp_desktop_namedview(dt))'. The #15307 fix bugs the 'dotted' checkbox for the XY-grid.
My code is buggy because it does not think about multiple documents: it should not use SP_ACTIVE_DESKTOP for example to retrieve the SPDocument, but it should use the repr to determine which document it is in.
I'll fixed this now. Thanks very much for uncovering this bug!!!
Cheers, Johan
participants (3)
-
unknown＠example.com -
bulia byak -
Diederik van Lierop