Bug 140220 - Replace SwClient with SvtListener in Writer
Summary: Replace SwClient with SvtListener in Writer
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard:
Keywords: difficultyMedium, easyHack, skillCpp, topicCleanup
Depends on:
Blocks: Dev-related
  Show dependency treegraph
 
Reported: 2021-02-06 20:44 UTC by Björn Michaelsen
Modified: 2024-03-04 15:41 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Björn Michaelsen 2021-02-06 20:44:07 UTC
Description:
SwClient is a very problematic implementation of the observer pattern(*). However these days all SwModify's are also a sw::BroadcasterMixin. This means the SwClient can be turned into a SvtListener without much effort to be notified. This EasyHack is about the following:

1/ find a direct derived class of SwClient (except SwModify/ListenerEntry)
   git grep public.*SwClient
2/ Instead of using SwClients Add/Remove/SWClientNotify/GetRegisteredIn, use GetNotifier/StartListening/EndListening
   an example of how this might be done can be found on gerrit:
   https://gerrit.libreoffice.org/c/core/+/110495



(*) more details can be found in this talk at LibOCon2018: https://www.youtube.com/watch?v=j6Znmaroe6M

Steps to Reproduce:
.

Actual Results:
.

Expected Results:
.


Reproducible: Always


User Profile Reset: No



Additional Info:
.
Comment 1 Harjot Singh 2021-12-04 10:30:41 UTC
Hi, I submitted a patch for this https://https://gerrit.libreoffice.org/c/core/+/125822
I took your example https://gerrit.libreoffice.org/c/core/+/110495 and made changes but the build failed. I am unable to progress further on this. Could you help me out on how to tackle this problem?
Comment 2 Harjot Singh 2021-12-04 10:33:09 UTC
(In reply to Harjot Singh from comment #1)
> Hi, I submitted a patch for this
> https://https://gerrit.libreoffice.org/c/core/+/125822
> I took your example https://gerrit.libreoffice.org/c/core/+/110495 and made
> changes but the build failed. I am unable to progress further on this. Could
> you help me out on how to tackle this problem?

Correction for link: https://gerrit.libreoffice.org/c/core/+/125822
Comment 3 Buovjaga 2021-12-04 12:36:22 UTC
(In reply to Harjot Singh from comment #2)
> (In reply to Harjot Singh from comment #1)
> > Hi, I submitted a patch for this
> > https://https://gerrit.libreoffice.org/c/core/+/125822
> > I took your example https://gerrit.libreoffice.org/c/core/+/110495 and made
> > changes but the build failed. I am unable to progress further on this. Could
> > you help me out on how to tackle this problem?
> 
> Correction for link: https://gerrit.libreoffice.org/c/core/+/125822

Looks like GetHeaderFormat() and GetFooterFormat() are used in many places (66 and 67 lines found in *.cxx files, respectively), so you would also need to modify those uses to match your changes in sw/inc/fmthdft.hxx

Maybe Björn can comment on this.
Comment 4 Devansh Varshney 2024-03-04 10:59:06 UTC
In file sw/inc/fmtpdsc.hxx -

at line 62 and 63 -

          SwPageDesc *GetPageDesc() { return static_cast<SwPageDesc*>(GetRegisteredIn()); }
    const SwPageDesc *GetPageDesc() const { return static_cast<const SwPageDesc*>(GetRegisteredIn()); }


I am bit confused -

SwPageDesc *m_pPageDesc = m_pPageDesc->GetNotifier();

or this ?

SwPageDesc *m_pPageDesc = static_cast<SwPageDesc*>(GetNotifier());

I also changed the-

    void RegisterToPageDesc( SwPageDesc& );

to 

    void RegisterToPageDesc( m_pPageDesc& );
Comment 5 Devansh Varshney 2024-03-04 13:15:02 UTC
(In reply to Devansh Varshney from comment #4)
> In file sw/inc/fmtpdsc.hxx -
> 
> at line 62 and 63 -
> 
>           SwPageDesc *GetPageDesc() { return
> static_cast<SwPageDesc*>(GetRegisteredIn()); }
>     const SwPageDesc *GetPageDesc() const { return static_cast<const
> SwPageDesc*>(GetRegisteredIn()); }

this returns a pointer to SwPageDesc object.
Comment 6 Devansh Varshney 2024-03-04 15:41:41 UTC
Currently having error with file sw/inc/swtable.hxx and need help with the below 
DynCastTable(), and GetInfo() error -



In file included from /home/devansh/libreoffice/sw/source/core/access/acccell.cxx:29:
/home/devansh/libreoffice/sw/inc/swtable.hxx:159:28: error: ‘virtual const SwTable* SwTable::DynCastTable() const’ marked ‘override’, but does not override
  159 |     virtual const SwTable* DynCastTable() const override { return this; }
      |                            ^~~~~~~~~~~~
/home/devansh/libreoffice/sw/inc/swtable.hxx:316:18: error: ‘virtual bool SwTable::GetInfo(SfxPoolItem&) const’ marked ‘override’, but does not override
  316 |     virtual bool GetInfo( SfxPoolItem& ) const override;
      |                  ^~~~~~~


as after replacing the SwClient with SvtListener at

class SW_DLLPUBLIC SwTable: public SvtListener          //Client of FrameFormat.

This was added in the below patch -

 virtual const SwTable* DynCastTable() const override { return this; }
 
https://gerrit.libreoffice.org/c/core/+/142014


> --------------------------------------------------------------

Also, I am rectifying this -

/home/devansh/libreoffice/sw/inc/swtable.hxx:210:64: error: cannot convert ‘SvtBroadcaster’ to ‘SwTableFormat*’ in initialization
  210 |     SwTableFormat* m_pTableFormat = m_pTableFormat->GetNotifier();
      |                                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
      |                                                                |
      |                                                                SvtBroadcaster
/home/devansh/libreoffice/sw/inc/swtable.hxx:398:37: error: ‘m_pTableFormat’ was not declared in this scope; did you mean ‘SwTableFormat’?
  398 |     SwFrameFormat* m_pFrameFormat = m_pTableFormat->GetNotifier();
      |                                     ^~~~~~~~~~~~~~
      |                                     SwTableFormat
/home/devansh/libreoffice/sw/inc/swtable.hxx:480:64: error: cannot convert ‘SvtBroadcaster’ to ‘SwFrameFormat*’ in initialization
  480 |     SwFrameFormat* m_pFrameFormat = m_pFrameFormat->GetNotifier();
      |                                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
      |                                                                |
      |                                                                SvtBroadcaster