https://bugs.documentfoundation.org/show_bug.cgi?id=96896
--- Comment #18 from akash...@gmail.com ---
Hi,
I have solved the bug.
Now, just want to make clear some details about this bug(warning:wall of text
ahead):
Although the original description of the bug is about writer crashing when the
title is changed from the api, the discussion revealed two new bugs. So in all
there were three bugs:
1. Writer crashes when title is changed from api.
2. Wrong numbers showing up in the title when changing from print preview and
back.
3. Even though read-only mode is not supported for an unsaved doc, writer
appends read-only when a document is opened with factory/swriter url.
So here is how the patch is working for each bug:
Bug 1. This is caused due to two lines (commented) in the patch. Some
observations: The controller appends to its title, the title of the model.
When a model disposes, it calls disposing in TitleHelper to release its
resources.
The call to dispose calls impl_sendTitleChangeEvent. Since a model title is a
subtitle for a controller, the controller calls for an update in its title.
The controller calls impl_updateTitleForController. Then in lines:
css::uno::Reference< css::frame::XTitle > xModelTitle(xController->getModel (),
css::uno::UNO_QUERY);
xModelTitle reference is NULL!! because the model is disposing...
if (!xModelTitle.is ())
xModelTitle.set(xController, css::uno::UNO_QUERY);
This line now calls the getTitle for a controller. and hence this recursion:
framework::TitleHelper::impl_updateTitleForController+0x81
framework::TitleHelper::impl_updateTitle+0x19d
framework::TitleHelper::getTitle+0x96
SfxBaseController::getTitle+0x4f
I don't know why these lines are here. I set a breakpoint at these lines and
ran through a series of events in writer. Then I checked the number of hits on
the breakpoint. Hits only occurred when the model was disposing.
Also peculiar is what follows after: (in short)
if (!xModelTitle.is ())
xModelTitle.set(xController, css::uno::UNO_QUERY);
if (xModelTitle.is())
{//code}
else
{//code}
Now if the xModelTitle is an empty reference, then it is made to refer to the
controller calling it. Then again there is a if clause which will always be
taken because we just set the reference!! There is also an else part which will
never be executed. Maybe someone can tell us what these lines were meant to
do..
Although it seems this is too much work just to break an infinite recursion, it
is not. I tried many variations all of which broke the loop, but also broke
some other part. One of this (appears in the patch) was that the model was
sending a title change event always, even if the title was not changed. Now
when the model was disposing, there was no title change, but still the
controller starts to update its subTitle and falls into recursion. In the
original code there is a reason for this which doesn't really help.
//can be changed after shared mode is supported as per UNO api
So, I didn't change this line.(although I tried experimenting and it broke
something else in the title).
Bug 2. As mentioned in earlier discussions, the XController was never releasing
its leasedNumber. This was identified early in the gdb sessions. But the real
problem was how to get the controller to release its resources(see comment 14).
I wanted to use the existing framework rather than writing new functions for
this case. I had observed that the controller sends a dispose event. So I
registered TitleHelper for it. The problem here was that casting XTitle into a
XListenerEvent reference directly is an ambiguous cast. So I have defined the
upcasting path:
xController->addEventListener(static_cast<css::lang::XEventListener*>
(static_cast< css::frame::XFrameActionListener* > (this)));
We can use dynamic_cast also but I think static will work in this case.
This easily fixes the problem as controller dispose event disposes the related
controllers resources. Also it requires minimum editing to the existing code
base.
Bug 3. It is required that a document be saved to toggle read-only mode. But
writer, if called from the api with a read-only option, loads it into edit mode
but still appends 'read-only' to the tittle. I went through the loadenv class
for this. And there url of types private:factory are given a CAN_BE_LOADED
classification. Because the content can be loaded in edit mode.
So should a exception be thrown (INVALID_MEDIADESCRIPTOR) or continue to load
in edit mode, but set read-only property as false?
imho nothing should happen without the knowledge of the programmer. So a
exception should be thrown. And ideally a file save
dialog(fileDialogHelper.cxx) should be opened after load environment is
initialized. Which opens a lot of other questions like when and how to throw
the dialog etc.
But I think that can be made in a different commit.
I wrote this up so that in the future, someone else need not spend much time
figuring out what is happening and help him/her to hunt down the bug faster.
Because I feel that their maybe a nasty edge-case hiding somewhere.
Please review the patch.
Also I plan to do the following:
1. Write unit tests for all the test cases I found.
2. titlehelper.cxx could use some documentation. So I plan to add that.
--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
Libreoffice-bugs mailing list
Libreoffice-bugs@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-bugs
