Re: [2/2] comctl32: Don't update the status if the item is no longer valid.

[Nikolay Sivov]

On 19.01.2016 17:45, Akihiro Sagawa wrote:
> This fixes https://bugs.winehq.org/show_bug.cgi?id=39677
> Signed-off-by: Akihiro Sagawa
>
> dlls/comctl32/listview.c       |  2 +-
>  dlls/comctl32/tests/listview.c | 22 ++++++++++++++++++++++
>  2 files changed, 23 insertions(+), 1 deletion(-)

I think this is correct, but to make sure I fully understand what's
going on. The idea is to return early from _SetItemT() for the case when
old focused item index is larger than current number items in a list?
> From reading surrounding code, it looks like application will receive

state notifications for item number that's no longer valid, am I right?
> - /* The focused item isn't updated after the invalidation */

The opposite is true, focused item is updated in this new test, from 0
to -1 (nothing focused), please fix this comment to reflect that.
> comctl32: Don't update the status if the item is no longer valid.

Subject needs some adjusting to, something like "Prevent invalid focuse
change notifications from LVS_OWNERDATA lists", or similar. Right now
it's not clear what 'status' and 'item' mean in this context.

Thanks for debugging this.

[2/2] comctl32: Don't update the status if the item is no longer valid.

[Akihiro Sagawa]

On Tue, 19 Jan 2016 18:43:13 +0300, Nikolay Sivov wrote:
> I think this is correct, but to make sure I fully understand what's
> going on. The idea is to return early from _SetItemT() for the case when
> old focused item index is larger than current number items in a list?
> From reading surrounding code, it looks like application will receive
> state notifications for item number that's no longer valid, am I right?

That's right. _SetItemT(), called from _SetItemState(), refuses updating
if nItem is out of range by checking infoPtr->nItemCount.
> > - /* The focused item isn't updated after the invalidation */
>
> The opposite is true, focused item is updated in this new test, from 0
> to -1 (nothing focused), please fix this comment to reflect that.

Sure.
> > comctl32: Don't update the status if the item is no longer valid.
>
> Subject needs some adjusting to, something like "Prevent invalid focuse
> change notifications from LVS_OWNERDATA lists", or similar. Right now
> it's not clear what 'status' and 'item' mean in this context.

OK, I'm going to use your sentence as the subject in try 2 patch.

Thanks for reviewing,
Akihiro Sagawa