Skip to content
This repository has been archived by the owner on Aug 14, 2019. It is now read-only.

Occasional incorrect bubble sizes - must implement hash? #631

Closed
walsh2000 opened this issue Nov 17, 2014 · 2 comments
Closed

Occasional incorrect bubble sizes - must implement hash? #631

walsh2000 opened this issue Nov 17, 2014 · 2 comments

Comments

@walsh2000
Copy link

@jessesquires

Occasionally I see heights of chat bubbles clearly wrong.
-- Typically the bubble for a single line of text is as tall as a bubble for an image, or an image is squished down to the height of a single line of text.

I suspected the CGSize cache (_messageBubbleCache) &/or cell reuse, because the incorrect size was typically when a larger bubble went offscreen because of an incoming chat message. So I investigated that angle. _messageBubbleCache uses messageItem.hash for the cache key. The demo application provides a custom implementation for -[hash], but my application uses custom classes for JSQMessageData.

I believe the problem is that my JSQMessageData does not implement hash, and therefore uses the NSObject implementation. It appears that the NSObject implementation is to return the object's pointer as the hash value. So, if one of my JSQMessageData objects representing a picture is deallocated and a single-line text is allocated at the same address, the cache will produce a false-positive hit.

This is clearly my problem with an obvious solution, but I was thinking that the JSQMessageViewController library could have helped me avoid this by making -[hash] a required method on the JSQMessageData protocol. (or define its own selector for the cache key)

What do you think about that as an enhancement request?

@jessesquires jessesquires changed the title Occasional incorrect bubble sizes Occasional incorrect bubble sizes - must implement hash? Nov 17, 2014
@jessesquires
Copy link
Owner

Good catch @walsh2000 !

I think making hash required is the best solution.

@jessesquires jessesquires added this to the Release 6.0.1 milestone Nov 17, 2014
@gblazex
Copy link

gblazex commented Nov 20, 2014

+1 it also happens if you are using asynchronous loading of images (in which case simply self.image.hash is not going to cut it).

michaelkirk pushed a commit to signalapp/Signal-iOS that referenced this issue Aug 15, 2016
TSAnimatedAdapter is used when rendering GIFs. TSAnimatedAdapter
inherits from JSQMediaItem but does not provide a custom implementation
of the hash method. The default implementation of hash in JSQMediaItem
results in all messages of a given interaction type (incoming,
outgoing) sharing a cached bubble size. For this reason, JSQMediaItem
subclasses are required to implement hash (see
jessesquires/JSQMessagesViewController#631).

This commit fixes issue #1275 by implementing hash in TSAnimatedAdapter
the same way TSPhotoAdapter does.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants