[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 13:49:59
Message-ID: 478b087a0907170649t6aa2417atad6d11ce47cef8ac () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
On Fri, Jul 17, 2009 at 2:41 PM, Sven Langkamp <sven.langkamp@gmail.com>wrote:
> 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.
>
Ah no, I was wrong here. Both patches do the same. Line type has the value
KoCharacterStyle::NoLineType by default so that case doesn't have to be
covered.
[Attachment #5 (text/html)]
<br><br><div class="gmail_quote">On Fri, Jul 17, 2009 at 2:41 PM, Sven Langkamp <span \
dir="ltr"><<a href="mailto:sven.langkamp@gmail.com">sven.langkamp@gmail.com</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;"> <div><div></div><div \
class="h5"><div class="gmail_quote">On Fri, Jul 17, 2009 at 1:48 PM, Thomas Zander \
<span dir="ltr"><<a href="mailto:zander@kde.org" \
target="_blank">zander@kde.org</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;">
On Friday 03 July 2009 03:43:16 Sven Langkamp wrote:<br>
> Hi,<br>
><br>
> in the ongoing plugfest testing an issue where text appeared with underline<br>
> and line-through in OOo has been found.<br>
> For details see<br>
> <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>
>ion-placeholder<br>
><br>
> I found out that the character style is loads with line type single if the<br>
> specified line type is empty and the style is "none"<br>
> I have attached a patch to change it to load as NoLineType. The ODF spec is<br>
> a bit confusing at this point, so I'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'm worried it doesn't cover all<br>
cases. Specifically the 'if()' directly following the one you modified might \
set<br> your style to 'solid' 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></div></div>- if (fixedType.isEmpty() \
&& !fixedStyle.isEmpty())<br>+ if (fixedType.isEmpty() && \
!fixedStyle.isEmpty() && fixedStyle != "none")<br> fixedType = \
"single";<br> <br><br>- if (fixedType.isEmpty() && \
!fixedStyle.isEmpty())<br>+ if (fixedStyle == "none")<br>+ \
fixedType.clear();<br>+ else if (fixedType.isEmpty() && \
!fixedStyle.isEmpty())<br> fixedType = "single";<br>
<br>Isn't your patch doing the exact opposite of my patch? I guess with need a \
unittest anyway.<br> </blockquote></div><br>Ah no, I was wrong here. Both patches do \
the same. Line type has the value KoCharacterStyle::NoLineType by default so that \
case doesn't have to be covered. <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