[Liferea-devel] item description=NULL

[Roman Beslik <rabeslik@gm...>]

Hi.

When I debugged some bug (read items become unread after update of a 
feed) I found another bug. If an item in an Atom feed has no "content" 
tag, the function itemset.c/itemset_generic_merge_check see that 
item_get_description(newItem) is NULL and item_get_description(oldItem) 
is "". I.e. you wrote NULL to the database, but you read "". This 
contradicts the common specification of a store: "you read what you wrote".

My patch changes Liferea such that item_get_description(oldItem) is 
NULL. However, the patch may introduce new bugs e.g. if description of 
an item must not be NULL in item.h/itemPtr/description . In such a case 
we should replace NULL with "" after parsing a feed.

Also the patch fixes that bug that read items become unread after update 
of a feed. The patch is appropriate for the source code in Git.

http://www.beroal.in.ua/prg/linux/liferea-merge-item.patch

Re: [Liferea-devel] item description=NULL

[Lars Lindner <lars.lindner@gm...>]

On Wed, Aug 10, 2011 at 6:15 PM, Roman Beslik <rabeslik@...> wrote:
> Hi.
>
> When I debugged some bug (read items become unread after update of a
> feed) I found another bug. If an item in an Atom feed has no "content"
> tag, the function itemset.c/itemset_generic_merge_check see that
> item_get_description(newItem) is NULL and item_get_description(oldItem)
> is "". I.e. you wrote NULL to the database, but you read "". This
> contradicts the common specification of a store: "you read what you wrote".
>
> My patch changes Liferea such that item_get_description(oldItem) is
> NULL. However, the patch may introduce new bugs e.g. if description of
> an item must not be NULL in item.h/itemPtr/description . In such a case
> we should replace NULL with "" after parsing a feed.
>
> Also the patch fixes that bug that read items become unread after update
> of a feed. The patch is appropriate for the source code in Git.
>
> http://www.beroal.in.ua/prg/linux/liferea-merge-item.patch

Hello Roman,

Thanks for taking the time to write the patch!

I'm not yet sure about merging. I still do not understand the real cause of
the unread items problem? How do they become unread again. Until
now I could not reproduce the problem.

Can you give some hints?

Best Regards,
Lars

Re: [Liferea-devel] item description=NULL

[Roman Beslik <rabeslik@gm...>]

On 23.08.11 01:39, Lars Lindner wrote:
> On Wed, Aug 10, 2011 at 6:15 PM, Roman Beslik<rabeslik@...>  wrote:
>> Hi.
>>
>> When I debugged some bug (read items become unread after update of a
>> feed) I found another bug.
>
> I'm not yet sure about merging. I still do not understand the real cause of
> the unread items problem? How do they become unread again. Until
> now I could not reproduce the problem.
Look at itemset.c/itemset_generic_merge_check. When we go to
{{{
if(oldItem->readStatus != newItem->readStatus)
	equal = FALSE;
}}}
and oldItem, newItem have the same identifier, then (oldItem->readStatus 
&& ! newItem->readStatus) is true. Therefore (equal == FALSE). Then DB 
is updated by the line:
{{{
oldItem->readStatus = newItem->readStatus;
}}}
I.e. after this (! oldItem->readStatus) is true.

I do not know why you can not reproduce. Maybe you have (! allowUpdates).

Re: [Liferea-devel] item description=NULL

[Lars Lindner <lars.lindner@gm...>]

On Tue, Aug 23, 2011 at 1:52 AM, Roman Beslik <rabeslik@...> wrote:
> On 23.08.11 01:39, Lars Lindner wrote:
>>
>> [...]
>
> and oldItem, newItem have the same identifier, then (oldItem->readStatus &&
> ! newItem->readStatus) is true. Therefore (equal == FALSE). Then DB is
> updated by the line:
> {{{
> oldItem->readStatus = newItem->readStatus;
> }}}
> I.e. after this (! oldItem->readStatus) is true.

Correct. Now I understand. This line was added recently for TinyTinyRss
and Google Reader updates to sync a remotely updated read status of the
subscription. Which of course doesn't make sense for normal subscriptions.

So the code is right for one type of subscriptions, but not for the other.

I'll see if I can think of a solution.

Cheers,
Lars

Re: [Liferea-devel] item description=NULL

[Roman Beslik <rabeslik@gm...>]

On 31.08.11 21:58, Lars Lindner wrote:
> On Tue, Aug 23, 2011 at 1:52 AM, Roman Beslik<rabeslik@...>  wrote:
>> On 23.08.11 01:39, Lars Lindner wrote:
>> and oldItem, newItem have the same identifier, then (oldItem->readStatus&&
>> ! newItem->readStatus) is true. Therefore (equal == FALSE). Then DB is
>> updated by the line:
>> {{{
>> oldItem->readStatus = newItem->readStatus;
>> }}}
>> I.e. after this (! oldItem->readStatus) is true.
>
> Correct. Now I understand. This line was added recently for TinyTinyRss
> and Google Reader updates to sync a remotely updated read status of the
> subscription. Which of course doesn't make sense for normal subscriptions.
>
> So the code is right for one type of subscriptions, but not for the other.
Never heard about TinyTinyRss before. Is the article below relevant?
http://tt-rss.org/redmine/wiki/tt-rss/JsonApiReference#updateArticle

Re: [Liferea-devel] item description=NULL

[Lars Lindner <lars.lindner@gm...>]

On Thu, Sep 1, 2011 at 9:08 AM, Roman Beslik <rabeslik@...> wrote:
> On 31.08.11 21:58, Lars Lindner wrote:
>>
>> On Tue, Aug 23, 2011 at 1:52 AM, Roman Beslik<rabeslik@...>  wrote:
>>>
>>> On 23.08.11 01:39, Lars Lindner wrote:
>>> and oldItem, newItem have the same identifier, then
>>> (oldItem->readStatus&&
>>> ! newItem->readStatus) is true. Therefore (equal == FALSE). Then DB is
>>> updated by the line:
>>> {{{
>>> oldItem->readStatus = newItem->readStatus;
>>> }}}
>>> I.e. after this (! oldItem->readStatus) is true.
>>
>> Correct. Now I understand. This line was added recently for TinyTinyRss
>> and Google Reader updates to sync a remotely updated read status of the
>> subscription. Which of course doesn't make sense for normal subscriptions.
>>
>> So the code is right for one type of subscriptions, but not for the other.
>
> Never heard about TinyTinyRss before. Is the article below relevant?
> http://tt-rss.org/redmine/wiki/tt-rss/JsonApiReference#updateArticle

No, this is the other direction. Imagine the use case where a user of
TinyTinyRss or Google Reader toggles the read state of the article
at the remote site. Then on the next update Liferea needs to sync
with this new read state.

Regards,
Lars

Re: [Liferea-devel] item description=NULL

[Lars Lindner <lars.lindner@gm...>]

On Thu, Sep 1, 2011 at 5:15 PM, Lars Lindner <lars.lindner@...> wrote:
> On Thu, Sep 1, 2011 at 9:08 AM, Roman Beslik <rabeslik@...> wrote:
> > On 31.08.11 21:58, Lars Lindner wrote:
> >> [...]
> > Never heard about TinyTinyRss before. Is the article below relevant?
> > http://tt-rss.org/redmine/wiki/tt-rss/JsonApiReference#updateArticle
>
> No, this is the other direction. Imagine the use case where a user of
> TinyTinyRss or Google Reader toggles the read state of the article
> at the remote site. Then on the next update Liferea needs to sync
> with this new read state.

Please note that git master now contains a fix for the problem.

Cheers,
Lars

Re: [Liferea-devel] item description=NULL

[Roman Beslik <rabeslik@gm...>]

On 07.09.11 19:30, Lars Lindner wrote:
> On Thu, Sep 1, 2011 at 5:15 PM, Lars Lindner<lars.lindner@...>  wrote:
>> On Thu, Sep 1, 2011 at 9:08 AM, Roman Beslik<rabeslik@...>  wrote:
>>> On 31.08.11 21:58, Lars Lindner wrote:
>>>> [...]
>>> Never heard about TinyTinyRss before. Is the article below relevant?
>>> http://tt-rss.org/redmine/wiki/tt-rss/JsonApiReference#updateArticle
>>
>> No, this is the other direction. Imagine the use case where a user of
>> TinyTinyRss or Google Reader toggles the read state of the article
>> at the remote site. Then on the next update Liferea needs to sync
>> with this new read state.
>
> Please note that git master now contains a fix for the problem.
Thank! The bug is fixed.