To comment on the following update, log in, then open the issue:
http://www.openoffice.org/issues/show_bug.cgi?id=108345
User mst changed the following:
What |Old value |New value
================================================================================
Status|NEW |STARTED
--------------------------------------------------------------------------------
------- Additional comments from m...@openoffice.org Thu Apr 1 14:05:49 +0000
2010 -------
hi dtardon,
finally had some time for your patch.
it's definitely a big improvement, but i still have some concerns:
did you have a look at the function lcl_IsNewAttrInSet() that is called?
it contains the condition:
( pOther->IsCharFmtAttr() || pOther->Which() == rItem.Which() )
the attribute set is initialized thus:
SfxItemSet aThisSet( GetDoc()->GetAttrPool(), aCharFmtSetRange );
which expands to (bastyp/init.cxx):
RES_CHRATR_BEGIN, RES_CHRATR_END-1,
RES_UNKNOWNATR_BEGIN, RES_UNKNOWNATR_END-1,
so the second part of the condition above compares the id of hints with the id
of items from aThisSet.
but since we have auto formats, there can never be hints with any of the types
in aCharFmtSetRange; instead there would be a hint with RES_TXTATR_AUTOFMT (such
as the one inserted by your patch).
as a result of this wrong condition the attributes from the paragraph could
overwrite the attributes of an existing auto format text hint.
[i am not sure if there can be a auto format hint that spans the entire
paragraph; maybe if FmtToTxtAttr is called on a node split path?]
furthermore, i don't understand why you copy the item set and introduced the
function lcl_CollectAttributes.
all it does is to copy the items via lcl_MergeAttr, and all that does is handle
RES_TXTATR_AUTOFMT items specially by expanding them.
but the item set that is copied can't contain RES_TXTATR_AUTOFMT in the first
place, right?
so this seems unnecessary to me.
oh, another nitpick:
please don't put pure whitespace changes into the patch, makes it difficult to
review.
only convert tabs on the lines that change anyway, and on adjacent trivialities
like { or }.
but eliminating evil BOOLs is always acceptable :)
i'd like to have this fixed in 3.3, so please give it another try (you're
definitely on the right track), or if you have something more important to do,
i'll try to fix it.
---------------------------------------------------------------------
Please do not reply to this automatically generated notification from
Issue Tracker. Please log onto the website and enter your comments.
http://qa.openoffice.org/issue_handling/project_issues.html#notification
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@sw.openoffice.org
For additional commands, e-mail: issues-h...@sw.openoffice.org
---------------------------------------------------------------------
To unsubscribe, e-mail: allbugs-unsubscr...@openoffice.org
For additional commands, e-mail: allbugs-h...@openoffice.org
