[prev in list] [next in list] [prev in thread] [next in thread] 

List:       koffice-devel
Subject:    Re: Patch for loading of line type in character styles
From:       Sven Langkamp <sven.langkamp () gmail ! com>
Date:       2009-07-17 12:41:17
Message-ID: 478b087a0907170541v3e34fd79ie1414c96ff4447e8 () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


On Fri, Jul 17, 2009 at 1:48 PM, Thomas Zander <zander@kde.org> wrote:

> On Friday 03 July 2009 03:43:16 Sven Langkamp wrote:
> > Hi,
> >
> > in the ongoing plugfest testing an issue where text appeared with
> underline
> > and line-through in OOo has been found.
> > For details see
> >
> http://plugtest.opendocsociety.org/doku.php?id=scenarios:20090623:presentat
> >ion-placeholder
> >
> > I found out that the character style is loads with line type single if
> the
> > specified line type is empty and the style is "none"
> > I have attached a patch to change it to load as NoLineType. The ODF spec
> is
> > a bit confusing at this point, so I'm posting it for review.
>
> Sorry for taking so long to reply...
>
> I think the patch is almost correct but I'm worried it doesn't cover all
> cases. Specifically the 'if()' directly following the one you modified
> might set
> your style to 'solid' instead of none.
> Can you test the attached patch to solve the issue you had too? I think it
> is
> a tad more complete.
>


-    if (fixedType.isEmpty() && !fixedStyle.isEmpty())
+    if (fixedType.isEmpty() && !fixedStyle.isEmpty() && fixedStyle !=
"none")
         fixedType = "single";


-    if (fixedType.isEmpty() && !fixedStyle.isEmpty())
+    if (fixedStyle == "none")
+        fixedType.clear();
+    else if (fixedType.isEmpty() && !fixedStyle.isEmpty())
         fixedType = "single";

Isn't your patch doing the exact opposite of my patch? I guess with need a
unittest anyway.

[Attachment #5 (text/html)]

<div class="gmail_quote">On Fri, Jul 17, 2009 at 1:48 PM, Thomas Zander <span \
dir="ltr">&lt;<a href="mailto:zander@kde.org">zander@kde.org</a>&gt;</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;"> On Friday 03 July 2009 03:43:16 \
Sven Langkamp wrote:<br> &gt; Hi,<br>
&gt;<br>
&gt; in the ongoing plugfest testing an issue where text appeared with underline<br>
&gt; and line-through in OOo has been found.<br>
&gt; For details see<br>
&gt; <a href="http://plugtest.opendocsociety.org/doku.php?id=scenarios:20090623:presentat" \
target="_blank">http://plugtest.opendocsociety.org/doku.php?id=scenarios:20090623:presentat</a><br>
 &gt;ion-placeholder<br>
&gt;<br>
&gt; I found out that the character style is loads with line type single if the<br>
&gt; specified line type is empty and the style is &quot;none&quot;<br>
&gt; I have attached a patch to change it to load as NoLineType. The ODF spec is<br>
&gt; a bit confusing at this point, so I&#39;m posting it for review.<br>
<br>
Sorry for taking so long to reply...<br>
<br>
I think the patch is almost correct but I&#39;m worried it doesn&#39;t cover all<br>
cases. Specifically the &#39;if()&#39; directly following the one you modified might \
set<br> your style to &#39;solid&#39; instead of none.<br>
Can you test the attached patch to solve the issue you had too? I think it is<br>
a tad more complete.<br>
</blockquote><div><br><br></div></div>-    if (fixedType.isEmpty() &amp;&amp; \
!fixedStyle.isEmpty())<br>+    if (fixedType.isEmpty() &amp;&amp; \
!fixedStyle.isEmpty() &amp;&amp; fixedStyle != &quot;none&quot;)<br>         \
fixedType = &quot;single&quot;;<br> <br><br>-    if (fixedType.isEmpty() &amp;&amp; \
!fixedStyle.isEmpty())<br>+    if (fixedStyle == &quot;none&quot;)<br>+        \
fixedType.clear();<br>+    else if (fixedType.isEmpty() &amp;&amp; \
!fixedStyle.isEmpty())<br>         fixedType = &quot;single&quot;;<br> <br>Isn&#39;t \
your patch doing the exact opposite of my patch? I guess with need a unittest \
anyway.<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]