Issue 74080 - ERROR: caught an exception (...) in DialogWindow::StoreData
Summary: ERROR: caught an exception (...) in DialogWindow::StoreData
Status: ACCEPTED
Alias: None
Product: General
Classification: Code
Component: ui (show other issues)
Version: 680m201
Hardware: All All
: P3 Trivial (vote)
Target Milestone: ---
Assignee: AOO issues mailing list
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-02-01 09:44 UTC by Frank Schönheit
Modified: 2013-08-07 15:31 UTC (History)
1 user (show)

See Also:
Issue Type: DEFECT
Latest Confirmation in: ---
Developer Difficulty: ---


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Frank Schönheit 2007-02-01 09:44:19 UTC
this is a follow-up of issue 74058. This bug was fixed by properly catching an
exception which happens in the scenario described below. However, there's still
something wrong that this code is ran through at all ...

- Tools / Macro / Organize Macros / OpenOffice.org Basic
- Edit the module "StarOffice Macros / FormWizard / DBMetaData"
  (or any other module in any other library, as long as it is *read-only* and
contains
  a dialog
=> the Basic IDE opens
- close the Basic IDE
=> ERROR: caught an exception (...) in DialogWindow::StoreData
Comment 1 Frank Schönheit 2007-02-01 10:03:09 UTC
fs->ab:
The assertion is because DialogWindow::StoreData tries to write its dialog,
which is vetoed because the library is read-only. However, normally, StoreData
should not attempt to write the dialog at all - it is not modified.
But it *assumes* it is modified because previously, it was declared to be
modified through calls of |SetDialogModelChanged(TRUE)|, with the following stack:
DlgEdObj::_propertyChange
DlgEdPropListenerImpl::propertyChange
cppu::OPropertySetHelper::fire
comphelper::OPropertySetAggregationHelper::propertiesChange
cppu::OPropertySetHelper::firePropertiesChangeEvent
UnoDialogControl::ImplUpdateResourceResolver
UnoDialogControl::ImplStartListingForResourceEvents
UnoDialogControl::setModel
sdr::contact::ViewObjectContactOfUnoControl_Impl::createControlForDevice
sdr::contact::ViewObjectContactOfUnoControl::getTemporaryControlForWindow
sdr::contact::ViewContactOfUnoControl::getTemporaryControlForWindow
SdrUnoObj::GetTemporaryControlForWindow
DlgEdObj::getFormDeviceInfo

So what happens here is:
The Basic/Dialog library is opened, during this, somebody requests the
FormDeviceInfo for a control. This needs to create a temporary control for the
dialog. This new (temporary) control, in its ImplUpdateResourceResolver, decides
to fire artificial property changes of all language dependent properties. On of
the listeners, the DlgEdObj, marks the dialog as modified because of this
(assumed) property change.

I think there are several problems here:
First, firing artificial property changes is a Bad Thing in general, at least
for my taste. It might trigger a lot (potentially) unwanted code. I'm not
completely sure about the reason for this firing, but isn't there a better way?

Second, the code for firing the artificial events looks like this:
  Reference< XMultiPropertySet >	xMultiPropSet( xPropertySet, ... );
  Reference< XPropertiesChangeListener > xListener( xPropertySet, ... );
  xMultiPropSet->firePropertiesChangeEvent( ..., xListener )
where xPropertySet is the dialog's model.
What you do here is using an implementation detail of the DialogControlModel: it
is derived from ::comphelper::OPropertySetAggregationHelper, which, by accident,
implements the XPropertiesChangeListener interface. One day, this base class
could be refactored so it doesn't support this interface anymore - in this case
your code would immediately crash.

I don't know which instance should be notified here, but it's certainly not the
model (xPropertySet) itself. The code only works because the dialog model (the
aggregation helper, respectively) in turn forwards the element to its own listeners.
Comment 2 ab 2007-02-14 15:52:17 UTC
STARTED
Comment 3 fredrik.haegg 2007-04-13 15:42:17 UTC
Hrm... I thought this issue was fixed and integrated in m206?
Comment 4 Frank Schönheit 2007-04-16 07:15:46 UTC
no. It's part of CWS ab34, which is not integrated, yet.
Comment 5 ab 2007-06-27 15:18:07 UTC
ab->cd: after having a closer look it seems to me the problem described
by fs is located inside the toolkit code, please have a look.
Comment 6 carsten.driesner 2007-07-02 07:45:01 UTC
cd: FS is right. I just misused the property change notifications to implement
the lanuage support. A better way would be to integrate a specific way to update
the language dependent properties.
Comment 7 carsten.driesner 2007-07-17 13:29:41 UTC
cd: I fear I have to change several classes to fix this issue correctly. Short
before code freeze this is not acceptable. Therefore I have to move this issue
to OOo 2.4.
Comment 8 carsten.driesner 2008-01-09 16:08:42 UTC
cd: Changed target, no more time to fix this issue.
Comment 9 Mathias_Bauer 2008-05-19 10:27:42 UTC
target 3.x