[Libreoffice] [PATCH] format clipboard

[Maxime de Roucy]

Hello,

Following my discussion with Cedric on IRC here is some other
explanations on why I chose to remove the "nSelectionType ==
nsSelectionType::SEL_TBL" case from the lcl_CreateEmptyItemSet function
in formatclipboard.cxx.

Without the patch : lcl_CreateEmptyItemSet is called only in the
formatclipboard.cxx file, at line 326, 399 and 511. At line 399, the
function is called with the nSelectionType parameter equal to
nsSelectionType::SEL_TBL. At the other place the function is called
with the nSelectionType parameter equal to the output of
"rWrtShell.GetSelectionType();". rWrtShell is a SwWrtShell. If you look
in the SwWrtShell::GetSelectionType function (line 1412 file
sw/source/ui/wrtsh/wrtsh1.cxx) you will see that when the program take
the only path where the function output can hold an
nsSelectionType::SEL_TBL attribut, the "shorter" function output is equal
to : "GetCntType() | nsSelectionType::SEL_TBL".
Then if you look in the SwEditShell::GetCntType function
(sw/source/core/edit/edws.cxx line 163) you will see this function HAVE
TO return something different from 0 : "OSL_ASSERT( nRet );"

Then we can say that the "rWrtShell.GetSelectionType();" instruction
can't return exactly "nsSelectionType::SEL_TBL". So the "nSelectionType
== nsSelectionType::SEL_TBL" part of the lcl_CreateEmptyItemSet
function is only call by the "lcl_CreateEmptyItemSet(
nsSelectionType::SEL_TBL, rPool );" instruction (line 399).

I think the "nSelectionType == nsSelectionType::SEL_TBL" part of the
lcl_CreateEmptyItemSet function is confusing so I moved it line 399
(just to recall : line 399, the only place where it is currently executed).

I hope this clarified a bit my choice to remove the "nSelectionType ==
nsSelectionType::SEL_TBL" part from the lcl_CreateEmptyItemSet
function.

Regards

Maxime de Roucy

-- 
Maxime de Roucy
Groupe LINAGORA - OSSA
80 rue Roque de Fillol
92800 PUTEAUX
Tel. : 0 810 251 251


Le vendredi 16 mars 2012 à 10:16 +0100, Maxime de Roucy a écrit :
> Here a copy of the mail I send to Cedric Bosdonnat… forgot to add the
> list as "Cc:".
> 
> Hello
> 
> Thanks you for asking me…
> 
> When you select a cell in Writer the selection type is a mix of
> nsSelectionType::SEL_TBL_CELLS, nsSelectionType::SEL_TBL and
> nsSelectionType::SEL_TXT.
> So when the lcl_CreateEmptyItemSet function is called directly with
> the nSelectionType (from rWrtShell.GetSelectionType()) the if
> statement "nSelectionType == nsSelectionType::SEL_TBL" is false.
> 
> So in the SwFormatClipboard::Copy function there is another
> statement : 
> 
> 
> 
> ______________________________________________________________________
> 
> 
> 
> 
> 	if( nSelectionType & nsSelectionType::SEL_TBL_CELLS )//only copy table attributes if really cells are selected (not only text in tables)
> 	{
> 		m_pTableItemSet = lcl_CreateEmptyItemSet( nsSelectionType::SEL_TBL, rPool );
> 		lcl_getTableAttributes( *m_pTableItemSet, rWrtShell );
> 	}
> 
> 
> 
> ______________________________________________________________________
> 
> 
> 
> 
> That call lcl_CreateEmptyItemSet with just nsSelectionType::SEL_TBL if
> the real selection type contain nsSelectionType::SEL_TBL_CELLS.
> 
> I found this way of doing a bit confusing and as tables parameters are
> handle separately in the whole format paintbrush code I thought that
> replacing : 
> 
> 
> 
> ______________________________________________________________________
> 
> 
> 
> 
> 	m_pTableItemSet = lcl_CreateEmptyItemSet( nsSelectionType::SEL_TBL, rPool );
> 
> 
> 
> ______________________________________________________________________
> 
> 
> 
> 
> by : 
> 
> 
> 
> ______________________________________________________________________
> 
> 
> 
> 
> 	m_pTableItemSet = new SfxItemSet(rPool,
> 		SID_ATTR_BORDER_INNER,  SID_ATTR_BORDER_SHADOW, //SID_ATTR_BORDER_OUTER is inbetween
> 		RES_BACKGROUND,         RES_SHADOW, //RES_BOX is inbetween
> 		SID_ATTR_BRUSH_ROW,     SID_ATTR_BRUSH_TABLE,
> 		RES_BREAK,              RES_BREAK,
> 		RES_PAGEDESC,           RES_PAGEDESC,
> 		RES_LAYOUT_SPLIT,       RES_LAYOUT_SPLIT,
> 		RES_ROW_SPLIT,          RES_ROW_SPLIT,
> 		RES_KEEP,               RES_KEEP,
> 		RES_FRAMEDIR,           RES_FRAMEDIR,
> 		FN_PARAM_TABLE_HEADLINE, FN_PARAM_TABLE_HEADLINE,
> 		FN_TABLE_BOX_TEXTORIENTATION, FN_TABLE_BOX_TEXTORIENTATION,
> 		FN_TABLE_SET_VERT_ALIGN, FN_TABLE_SET_VERT_ALIGN,
> 		0);
> 
> 
> 
> ______________________________________________________________________
> 
> 
> 
> 
> was not irrelevant.
> 
> After this replacement was made there where no reasons keeping the
> "nSelectionType == nsSelectionType::SEL_TBL" block in the
> lcl_CreateEmptyItemSet function.
> 
> I hope I answered your question.
> 
> BEWARE : this patch need the first patch
> 0001-add-GetCurParAttr-and-GetPaMParAttr-in-SwEditShell from my
> preview mail
> http://lists.freedesktop.org/archives/libreoffice/2012-March/028157.html
> 
> Regards
> 
> Maxime de Roucy
> 
> _______________________________________________
> LibreOffice mailing list
> LibreOffice at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libreoffice

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/libreoffice/attachments/20120327/580d1ffd/attachment-0001.pgp>