[prev in list] [next in list] [prev in thread] [next in thread]
List: koffice-devel
Subject: Re: koffice/libs/kotext
From: Pierre Stirnweiss <pstirnweiss () googlemail ! com>
Date: 2009-02-27 9:52:30
Message-ID: af76e3dc0902270152i172737c6q10950a4648b82c04 () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
On Fri, Feb 27, 2009 at 7:14 AM, Thorsten Zachmann <t.zachmann@zagge.de>wrote:
> Hello,
>
> On Thursday 26 February 2009, Pierre Stirnweiss wrote:
> > I quickly had a look and see your point. I actually think that the styles
> > should nevertheless get the default properties in cstr.
>
> I don't think that is a good idea. The styles are created in the way that
> they
> only contain the attributes they change. I don't think we should make a
> difference for some values.
>
To be honest, I am not too sure any more about what I think about this.
There are two sort of conflicting use case which need to be handled, and our
current implementation works only for the first:
- the first use case is the following: the user have a current style he is
typing text with, lets say DjVU 12, underlined. He wants to highlight a word
he is about to type and hit bold (or goes to char format dia and set bold).
He type the to-be-highlighted word. He is indeed expecting, in that case,
that only the changed property is applied, meaning he expects DjVu 12,
underlined, bold. Here we have inheritance from the previous char.
This is what our current implementation of style does (I am speaking of
KoParagStyle and KoCharStyles, not the KoXmlElement styles when loading).
- the second use case, which is broken currently is: The user has defined a
named style called "Basic" with properties Sans Serif, 14 and another named
style called "Basic Highlight" inheriting the "Basic" style and adding the
property bold. The "Basic Highlight" should not set the font family and
size, because he wants to have the Basic Highlight follow the parent style,
just it being bold.
Now he is typing in DjVu 12 some text and apply the style "basic Highlight"
for some new text he want to type. His expectation would be that the new
text he will type will be Sans Serif 14 bold. With our current system, the
text he will enter will be DjVu 12 bold. We actually already have at least
one bug report on this for KWord (bug nbr I can't find right now).
This is why I am saying that we only have partial style inheritance (because
style properties from parent are copied at loading, but this is a "cp" stuff
not a "ln -s" stuff).
> > The logic of
> > inheriting the previous span properties is an ODF logic and should be
> > handled in the ODF loading mechanism.
>
Now, this is actually not even true. I had a look at the spec (1.2 draft,
but I think it didn't change):
a <text:span> is a piece of text which applies *one* style
copied from the spec:
The <text:span> element represents portions of text using a particular
style. The content of this element is the text which uses that text style.
and:
Styles defined by the <style:style> element create a hierarchical style
model and support inheritance of formatting properties. To determine the
value of a formatting property, it is first checked if the style that is
referenced by a particular object directly, for instance by a paragraph,
specifies a value for the formatting property. If this is the case, then
this value is taken. If the style does not specify a value for the
formatting property, and if the style has a parent style, then this parent
style is checked. A parent style is specified the style:parent-style-name
attribute.
If the parent style specifies a value for the formatting property, this
value is taken. Otherwise its parent style is checked, if it is defined.
This process is continued until either a value for the formatting property
has been found, or until a style has no parent style. If a value for the
formatting property has not been found, then the default style (see 15.3)
that has the same family as the style that has been referenced initially is
checked. If it specifies a value for the formatting property, then this
value is taken. Otherwise an implementation specific value is taken.
This means that inheriting from the previous span is *not* foreseen in the
spec.
> This would not be very difficult: in
> > load span, we store the current cursor charFormat before getting the
> > properties of the current span format. We could check the previous char
> > properties and overload the default with these if set.
>
> What is the problem you try to fix by hardcoding some values the style?
> From
> the commit message I get it is used in case defaultstyles.xml is not found.
> Would it not be better to provide a koffice wide defaultstyle.xml and use
> that in case an application specific one is not found? In any case koffice
> should always have a default style to save back that has the correct
> values.
> With that also saving should be fixed.
I am not sure what you refer to when saying it would fix saving. If you are
referring to my commit message, this part of the patch should go in no
matter what, because we will have problems at a certain point in time: We
are comparing the QTextFragment char properties with the char properties of
the "original" style given by the styleManager to see if the fragment
dirtied the style. We remove the properties given by the blockCharFormat
(because these will be saved in the "paragraph" style, and not as a "text"
style in the odf). The problem is that we remove these from the
cursor.charFormat and not from the originalStyle charFormat. We are
therefore comparing apples and oranges there. These properties should be
removed from both cursor.charFormat and originalCharFormat before comparing.
As far as the: what problem are you trying to solve there, they are actually
two-fold:
- first, currently if we create a new style in the styleManager, it will not
inherit the defaultstyles.xml properties. there is a risk our default values
are not set. Which means that this new style will rely on Qt default (which
can be set system wide and can be different from ours), whereas the loaded
styles are relying on defaultstyles.xml defaults.
- second, should something (script or external apps) use kotext/textshape
with a created QTextDoc, and styles, without using the whole odf loading
mechanism (and as far as I can see, this includes our loadXML mechanism for
loading 1.6 docs), then the defaultstyles.xml are not available (even if the
file is actually there). Plus, do we really want to force let's say Krita to
load always, default text styles just in case they might include a textshape
layer?, what about default properties of KSpread,....
>
> What do you think?
So in conclusion, I think that:
- the first use case has actually nothing to do with odf loading, since as
far as I understand the ODF spec, inheriting from the previous span is not
foreseen.
- the second use case and loading would actually benefit from having a
default style set in cstr. The second use case would actually require deeper
modifications to our style inheritance handling.
As far as your given ODF example, it seems to me that if T2 does not specify
a font, it should be the parent's font or, in case no parent's font, the
application default font which should be applied, and not the previous span
style. Should T2 wan the same font as the previous span, it should also
specify it (if different from default).
Pierre
[Attachment #5 (text/html)]
<br><br><div class="gmail_quote">On Fri, Feb 27, 2009 at 7:14 AM, Thorsten Zachmann \
<span dir="ltr"><<a href="mailto:t.zachmann@zagge.de" \
target="_blank">t.zachmann@zagge.de</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt \
0pt 0.8ex; padding-left: 1ex;">
Hello,<br>
<div><br>
On Thursday 26 February 2009, Pierre Stirnweiss wrote:<br>
> I quickly had a look and see your point. I actually think that the styles<br>
> should nevertheless get the default properties in cstr.<br>
<br>
</div>I don't think that is a good idea. The styles are created in the way that \
they<br> only contain the attributes they change. I don't think we should make \
a<br> difference for some values.<br>
<div></div></blockquote><div><br>To be honest, I am not too sure any more about what \
I think about this. There are two sort of conflicting use case which need to be \
handled, and our current implementation works only for the first:<br>
<br>- the first use case is the following: the user have a current style he is typing \
text with, lets say DjVU 12, underlined. He wants to highlight a word he is about to \
type and hit bold (or goes to char format dia and set bold). He type the \
to-be-highlighted word. He is indeed expecting, in that case, that only the changed \
property is applied, meaning he expects DjVu 12, underlined, bold. Here we have \
inheritance from the previous char.<br>
This is what our current implementation of style does (I am speaking of KoParagStyle \
and KoCharStyles, not the KoXmlElement styles when loading).<br><br>- the second use \
case, which is broken currently is: The user has defined a named style called \
"Basic" with properties Sans Serif, 14 and another named style called \
"Basic Highlight" inheriting the "Basic" style and adding the \
property bold. The "Basic Highlight" should not set the font family and \
size, because he wants to have the Basic Highlight follow the parent style, just it \
being bold.<br>
Now he is typing in DjVu 12 some text and apply the style "basic Highlight" \
for some new text he want to type. His expectation would be that the new text he will \
type will be Sans Serif 14 bold. With our current system, the text he will enter will \
be DjVu 12 bold. We actually already have at least one bug report on this for KWord \
(bug nbr I can't find right now).<br>
This is why I am saying that we only have partial style inheritance (because style \
properties from parent are copied at loading, but this is a "cp" stuff not \
a "ln -s" stuff).<br><br></div><blockquote class="gmail_quote" \
style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; \
padding-left: 1ex;">
<div><br>
> The logic of<br>
> inheriting the previous span properties is an ODF logic and should be<br>
> handled in the ODF loading mechanism. </div></blockquote><div><br>Now, this is \
actually not even true. I had a look at the spec (1.2 draft, but I think it \
didn't change):<br><br>a <text:span> is a piece of text which applies \
<u>one</u> style<br> copied from the spec:<br><meta http-equiv="Content-Type" \
content="text/html; charset=utf-8"><meta name="ProgId" content="Word.Document"><meta \
name="Generator" content="Microsoft Word 11"><meta name="Originator" \
content="Microsoft Word 11"><link rel="File-List" \
href="file:///C:%5CDOCUME%7E1%5Cps51311%5CLOCALS%7E1%5CTemp%5Cmsohtml1%5C01%5Cclip_filelist.xml"><style>
<!--
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{mso-style-parent:"";
margin-top:4.0pt;
margin-right:0cm;
margin-bottom:4.0pt;
margin-left:0cm;
mso-pagination:widow-orphan no-line-numbers;
mso-hyphenate:none;
font-size:10.0pt;
mso-bidi-font-size:12.0pt;
font-family:Arial;
mso-fareast-font-family:"Times New Roman";
mso-bidi-font-family:"Times New Roman";
mso-font-kerning:.5pt;
mso-ansi-language:EN-US;
mso-fareast-language:AR-SA;}
p.MsoBodyText, li.MsoBodyText, div.MsoBodyText
{margin-top:4.0pt;
margin-right:0cm;
margin-bottom:4.0pt;
margin-left:0cm;
mso-pagination:widow-orphan no-line-numbers;
mso-hyphenate:none;
font-size:10.0pt;
mso-bidi-font-size:12.0pt;
font-family:Arial;
mso-fareast-font-family:"Times New Roman";
mso-bidi-font-family:"Times New Roman";
mso-font-kerning:.5pt;
mso-ansi-language:EN-US;
mso-fareast-language:AR-SA;}
span.Element
{mso-style-name:Element;
mso-ansi-font-size:10.0pt;
font-family:"Courier New";
mso-ascii-font-family:"Courier New";
mso-fareast-font-family:"Times New Roman";
mso-hansi-font-family:"Courier New";
mso-bidi-font-family:"Courier New";
mso-ansi-language:EN-US;
mso-fareast-language:X-NONE;}
@page Section1
{size:612.0pt 792.0pt;
margin:72.0pt 90.0pt 72.0pt 90.0pt;
mso-header-margin:36.0pt;
mso-footer-margin:36.0pt;
mso-paper-source:0;}
div.Section1
{page:Section1;}
-->
</style>
<p class="MsoNormal"><font size="2"><span lang="EN-US">The <text:span> element \
represents portions of text using a particular style. The content of this element is \
the text which uses that text style.</span></font></p>
and:<br><meta http-equiv="Content-Type" content="text/html; charset=utf-8"><meta \
name="ProgId" content="Word.Document"><meta name="Generator" content="Microsoft Word \
11"><meta name="Originator" content="Microsoft Word 11"><link rel="File-List" \
href="file:///C:%5CDOCUME%7E1%5Cps51311%5CLOCALS%7E1%5CTemp%5Cmsohtml1%5C02%5Cclip_filelist.xml"><style>
<!--
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{mso-style-parent:"";
margin-top:4.0pt;
margin-right:0cm;
margin-bottom:4.0pt;
margin-left:0cm;
mso-pagination:widow-orphan no-line-numbers;
mso-hyphenate:none;
font-size:10.0pt;
mso-bidi-font-size:12.0pt;
font-family:Arial;
mso-fareast-font-family:"Times New Roman";
mso-bidi-font-family:"Times New Roman";
mso-font-kerning:.5pt;
mso-ansi-language:EN-US;
mso-fareast-language:AR-SA;}
p.MsoBodyText, li.MsoBodyText, div.MsoBodyText
{margin-top:4.0pt;
margin-right:0cm;
margin-bottom:6.0pt;
margin-left:0cm;
mso-pagination:widow-orphan no-line-numbers;
mso-hyphenate:none;
font-size:10.0pt;
mso-bidi-font-size:12.0pt;
font-family:Arial;
mso-fareast-font-family:"Times New Roman";
mso-bidi-font-family:"Times New Roman";
mso-font-kerning:.5pt;
mso-ansi-language:EN-US;
mso-fareast-language:AR-SA;}
span.Element
{mso-style-name:Element;
mso-ansi-font-size:10.0pt;
font-family:"Courier New";
mso-ascii-font-family:"Courier New";
mso-fareast-font-family:"Times New Roman";
mso-hansi-font-family:"Courier New";
mso-bidi-font-family:"Courier New";
mso-ansi-language:EN-US;
mso-fareast-language:X-NONE;}
span.Attribute
{mso-style-name:Attribute;
mso-ansi-font-size:10.0pt;
font-family:"Courier New";
mso-ascii-font-family:"Courier New";
mso-fareast-font-family:"Times New Roman";
mso-hansi-font-family:"Courier New";
mso-bidi-font-family:"Courier New";
mso-ansi-language:EN-US;
mso-fareast-language:X-NONE;}
p.Changed1, li.Changed1, div.Changed1
{mso-style-name:Changed1;
mso-style-parent:"Body Text";
margin-top:4.0pt;
margin-right:0cm;
margin-bottom:4.0pt;
margin-left:0cm;
mso-pagination:widow-orphan no-line-numbers;
mso-hyphenate:none;
background:yellow;
font-size:10.0pt;
mso-bidi-font-size:12.0pt;
font-family:Arial;
mso-fareast-font-family:"Times New Roman";
mso-bidi-font-family:"Times New Roman";
mso-font-kerning:.5pt;
mso-ansi-language:EN-US;
mso-fareast-language:AR-SA;}
@page Section1
{size:612.0pt 792.0pt;
margin:72.0pt 90.0pt 72.0pt 90.0pt;
mso-header-margin:36.0pt;
mso-footer-margin:36.0pt;
mso-paper-source:0;}
div.Section1
{page:Section1;}
-->
</style>
<p class="MsoNormal"><font size="2"><span lang="EN-US">Styles defined by the \
<style:style> element create a hierarchical style model and support inheritance \
of formatting properties. To determine the value of a formatting property, it is \
first checked if the style that is referenced by a particular object directly, for
instance by a paragraph, specifies a value for the formatting property. If this
is the case, then this value is taken. If the style does not specify a value
for the formatting property, and if the style has a parent style, then this
parent style is checked. A parent style is specified the style:parent-style-name
attribute.<a name="line-19"></a><a name="line-18"></a></span></font></p>
<p class="MsoNormal"><font size="2"><span lang="EN-US">If the parent style specifies \
a value for the formatting property, this value is taken. Otherwise its parent style \
is checked, if it is defined. This process is continued until either a value for the
formatting property has been found, or until a style has no parent style. If a
value for the formatting property has not been found, then the default style
(see <span style="">15.3</span>)
that has the same family as the style that has been referenced initially is
checked. If it specifies a value for the formatting property, then this value
is taken. Otherwise an implementation specific value is taken.</span></font></p>
<br>This means that inheriting from the previous span is <u>not</u> foreseen in the \
spec.<br><br> </div><blockquote class="gmail_quote" style="border-left: 1px solid \
rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> <div>This would \
not be very difficult: in<br> > load span, we store the current cursor charFormat \
before getting the<br> > properties of the current span format. We could check the \
previous char<br> > properties and overload the default with these if set.<br>
<br>
</div>What is the problem you try to fix by hardcoding some values the style? \
From<br> the commit message I get it is used in case defaultstyles.xml is not \
found.<br> Would it not be better to provide a koffice wide defaultstyle.xml and \
use<br> that in case an application specific one is not found? In any case \
koffice<br> should always have a default style to save back that has the correct \
values.<br> With that also saving should be fixed.</blockquote><div><br>I am not sure \
what you refer to when saying it would fix saving. If you are referring to my commit \
message, this part of the patch should go in no matter what, because we will have \
problems at a certain point in time: We are comparing the QTextFragment char \
properties with the char properties of the "original" style given by the \
styleManager to see if the fragment dirtied the style. We remove the properties given \
by the blockCharFormat (because these will be saved in the "paragraph" \
style, and not as a "text" style in the odf). The problem is that we remove \
these from the cursor.charFormat and not from the originalStyle charFormat. We are \
therefore comparing apples and oranges there. These properties should be removed from \
both cursor.charFormat and originalCharFormat before comparing.<br> <br>As far as \
the: what problem are you trying to solve there, they are actually two-fold:<br>- \
first, currently if we create a new style in the styleManager, it will not inherit \
the defaultstyles.xml properties. there is a risk our default values are not set. \
Which means that this new style will rely on Qt default (which can be set system wide \
and can be different from ours), whereas the loaded styles are relying on \
defaultstyles.xml defaults.<br>
- second, should something (script or external apps) use kotext/textshape with a \
created QTextDoc, and styles, without using the whole odf loading mechanism (and as \
far as I can see, this includes our loadXML mechanism for loading 1.6 docs), then the \
defaultstyles.xml are not available (even if the file is actually there). Plus, do we \
really want to force let's say Krita to load always, default text styles just in \
case they might include a textshape layer?, what about default properties of \
KSpread,....<br> <br></div><blockquote class="gmail_quote" style="border-left: 1px \
solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><br> <br>
What do you think?</blockquote><div><br>So in conclusion, I think that:<br>- the \
first use case has actually nothing to do with odf loading, since as far as I \
understand the ODF spec, inheriting from the previous span is not \
foreseen.<br>
- the second use case and loading would actually benefit from having a default style \
set in cstr. The second use case would actually require deeper modifications to our \
style inheritance handling.<br><br>As far as your given ODF example, it seems to me \
that if T2 does not specify a font, it should be the parent's font or, in case no \
parent's font, the application default font which should be applied, and not the \
previous span style. Should T2 wan the same font as the previous span, it should also \
specify it (if different from default).<br> <br>Pierre<br></div></div><br>
_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic