On Saturday 03 January 2009 06:36:35 pm Brad Hards wrote:
> http://bugs.kde.org/show_bug.cgi?id=154980
> 1. We aren't handling the different alignment of elements within a single
> cell. So the first cell in the second row should render something like:
> BOH
> MAH
> CHI LO SA
> But we render it as
> CHI LO SAMAHBOH (with some wordwrapping)
> That is caused partly by inserting text at the beginning position
> (cell.firstCursorPosition(), which is easily fixed) and partly by not being
> able to set the alignment as a text property, only as a block property. The
> only way I can see of resolving this is to insert invisible frames within
> the cell, one for each <p> block.
See patch below.
> 3. We don't properly size the cells / columns. Need to parse the
> table-column attributes, and find a way to map that into a Qt property.
> Presumably that will require some kind of call to
> tableFormat.setColumnWidthConstraints().
This was actually implemented, but had a bug. I think it is caused by calling
the parent (which doesn't implement the column width) which adds a row size
of zero, followed by the actual column property. The whole approach is a bit
brittle, but I've worked around it. See below.
> 4. We don't support lists. Need to support the parsing and list styles.
Well, we don't support lists within table cells.
5. I also found that if you have anything after the table, all that gets put into
the first table cell. This is really noticeable if you have a couple of tables.
I've attached a test case that shows this.
Those problems are (on my testing) all resolved by this change. Lines that
start with ##### BH: are to help explain the patch.
This is better on my testing than currently, and I think it should go into
4.2 as a bug fix.
Brad
Index: formatproperty.cpp
===================================================================
--- formatproperty.cpp (revision 905238)
+++ formatproperty.cpp (working copy)
@@ -370,12 +370,15 @@
}
##### BH: Changes to formatproperty.cpp are to deal with the problem where we
##### BH: add column widths that are zero, presumably because of being called
##### BH: from a parent that doesn't set it.
TableColumnFormatProperty::TableColumnFormatProperty()
- : mWidth( 0 )
+ : mWidth( 0 ), isValid( false )
{
}
void TableColumnFormatProperty::apply( QTextTableFormat *format ) const
{
+ if ( ! isValid ) {
+ return;
+ }
QVector<QTextLength> lengths = format->columnWidthConstraints();
lengths.append( QTextLength( QTextLength::FixedLength, mWidth ) );
@@ -385,6 +388,7 @@
void TableColumnFormatProperty::setWidth( double width )
{
mWidth = width;
+ isValid = true;
}
TableCellFormatProperty::TableCellFormatProperty()
Index: formatproperty.h
===================================================================
--- formatproperty.h (revision 905238)
+++ formatproperty.h (working copy)
@@ -173,6 +173,7 @@
##### BH: Just add the member variable for the validity check
private:
double mWidth;
+ bool isValid;
};
class TableCellFormatProperty
Index: converter.cpp
===================================================================
--- converter.cpp (revision 903994)
+++ converter.cpp (working copy)
@@ -195,7 +195,7 @@
##### BH: We need to be able to insert the list at a variable cursor position (not
##### BH: just mCursor), in order to put the list into the right cell / frame location.
##### BH: So we pass the cursor position as a parameter.
##### BH:
##### BH: This change just makes the old list handling work the same as before.
if ( !convertHeader( mCursor, child ) )
return false;
} else if ( child.tagName() == QLatin1String( "list" ) ) {
- if ( !convertList( child ) )
+ if ( !convertList( mCursor, child ) )
return false;
} else if ( child.tagName() == QLatin1String( "table" ) ) {
if ( !convertTable( child ) )
@@ -321,21 +321,21 @@
return true;
}
##### BH: Insert list at cursor position passed as argument.
-bool Converter::convertList( const QDomElement &element )
+bool Converter::convertList( QTextCursor *cursor, const QDomElement &element )
{
const QString styleName = element.attribute( "style-name" );
const ListFormatProperty property = mStyleInformation->listProperty( styleName );
QTextListFormat format;
- if ( mCursor->currentList() ) { // we are in a nested list
- format = mCursor->currentList()->format();
+ if ( cursor->currentList() ) { // we are in a nested list
+ format = cursor->currentList()->format();
format.setIndent( format.indent() + 1 );
}
property.apply( &format, 0 );
- QTextList *list = mCursor->insertList( format );
+ QTextList *list = cursor->insertList( format );
QDomElement itemChild = element.firstChildElement();
int loop = 0;
@@ -350,17 +350,17 @@
if ( childElement.tagName() == QLatin1String( "p" ) ) {
if ( loop > 1 )
- mCursor->insertBlock();
+ cursor->insertBlock();
- prevBlock = mCursor->block();
+ prevBlock = cursor->block();
- if ( !convertParagraph( mCursor, childElement, QTextBlockFormat(), true ) )
+ if ( !convertParagraph( cursor, childElement, QTextBlockFormat(), true ) )
return false;
} else if ( childElement.tagName() == QLatin1String( "list" ) ) {
- prevBlock = mCursor->block();
+ prevBlock = cursor->block();
- if ( !convertList( childElement ) )
+ if ( !convertList( cursor, childElement ) )
return false;
}
@@ -391,6 +391,7 @@
*/
int rowCounter = 0;
int columnCounter = 0;
+
QQueue<QDomNode> nodeQueue;
enqueueNodeList( nodeQueue, element.childNodes() );
while ( !nodeQueue.isEmpty() ) {
@@ -420,6 +421,7 @@
* Create table
*/
QTextTable *table = mCursor->insertTable( rowCounter, columnCounter );
##### BH: Adding a table leaves the cursor in the first cell. We want to skip
##### BH: over that, and add additional text at the bottom.
+ mCursor->movePosition( QTextCursor::End );
/**
* Fill table
@@ -450,11 +452,24 @@
##### BH: This change adds an invisible frame around the text provided in each
##### BH: <p> block, so we can vary the block formatting (e.g. left, right, center
##### BH: alignment.
##### BH: I renamed the cursor so I can figure out _which_ cursor is being used...
while ( !paragraphElement.isNull() ) {
if ( paragraphElement.tagName() == QLatin1String( "p" ) ) {
QTextTableCell cell = table->cellAt( rowCounter, columnCounter );
- QTextCursor cursor = cell.firstCursorPosition();
- cursor.setBlockFormat( format );
+ // Insert a frame into the cell and work on that, so we can handle
+ // different parts of the cell having different block formatting
##### BH: Add new text at the end of the cell, not at the start.
+ QTextCursor cellCursor = cell.lastCursorPosition();
+ QTextFrameFormat frameFormat;
+ frameFormat.setMargin( 1 ); // TODO: this shouldn't be hard coded
+ QTextFrame *frame = cellCursor.insertFrame( frameFormat );
+ QTextCursor frameCursor = frame->firstCursorPosition();
+ frameCursor.setBlockFormat( format );
- if ( !convertParagraph( &cursor, paragraphElement, format ) )
+ if ( !convertParagraph( &frameCursor, paragraphElement, format ) )
return false;
##### BH: Previously we only handled text (<p>) within a cell. This adds support
##### BH: for lists with a cell. Re-uses existing list handling, just with a special
##### BH: cursor position. This is the reason for the extra parameter introduced
##### BH: into convertList, above.
+ } else if ( paragraphElement.tagName() == QLatin1String( "list" ) ) {
+ QTextTableCell cell = table->cellAt( rowCounter, columnCounter );
+ // insert a list into the cell
+ QTextCursor cellCursor = cell.lastCursorPosition();
+ if ( !convertList( &cellCursor, paragraphElement ) ) {
+ return false;
+ }
}
paragraphElement = paragraphElement.nextSiblingElement();
@@ -467,7 +482,11 @@
rowCounter++;
} else if ( el.tagName() == QLatin1String( "table-column" ) ) {
##### BH: This change deals with the "number-columns-repeated" approach to column width.
##### BH: Prova.odt uses that.
const StyleFormatProperty property = mStyleInformation->styleProperty( el.attribute( "style-name" ) );
- property.applyTableColumn( &tableFormat );
+ const QString tableColumnNumColumnsRepeated = el.attribute( "number-columns-repeated", "1" );
+ int numColumnsToApplyTo = tableColumnNumColumnsRepeated.toInt();
+ for (int i = 0; i < numColumnsToApplyTo; ++i) {
+ property.applyTableColumn( &tableFormat );
+ }
}
}
Index: converter.h
===================================================================
--- converter.h (revision 903994)
+++ converter.h (working copy)
@@ -40,7 +40,7 @@
##### BH: we changed the signature for convertList - just update here too.
bool convertTextNode( QTextCursor *cursor, const QDomText &element, const QTextCharFormat &format );
bool convertSpan( QTextCursor *cursor, const QDomElement &element, const QTextCharFormat &format );
bool convertLink( QTextCursor *cursor, const QDomElement &element, const QTextCharFormat &format );
- bool convertList( const QDomElement &element );
+ bool convertList( QTextCursor *cursor, const QDomElement &element );
bool convertTable( const QDomElement &element );
bool convertFrame( const QDomElement &element );
bool convertAnnotation( QTextCursor *cursor, const QDomElement &element );
-------------- next part --------------
A non-text attachment was scrubbed...
Name: table-test.odt
Type: application/vnd.oasis.opendocument.text
Size: 8144 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/okular-devel/attachments/20090104/8145bf7d/attachment-0001.odt