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

Properly copy attributes before modifying in JSQMessagesCollectionViewFlowLayout #1868

Merged
merged 1 commit into from
Oct 18, 2016

Conversation

algrid
Copy link
Contributor

@algrid algrid commented Oct 16, 2016

Pull request checklist

  • All tests pass. Demo project builds and runs.
  • I have resolved any merge conflicts.
  • I have followed the coding style, and reviewed the contributing guidelines. Confirmation: ____

This fixes issue #1218

What's in this pull request?

Ensure that attributes array is properly copied before modification.
Could also fix #1424 but I'm not sure about that.

@algrid algrid mentioned this pull request Oct 16, 2016
@codecov-io
Copy link

codecov-io commented Oct 16, 2016

Current coverage is 61.27% (diff: 50.00%)

Merging #1868 into develop will not change coverage

@@            develop      #1868   diff @@
==========================================
  Files            62         62          
  Lines          2249       2249          
  Methods         608        608          
  Messages          0          0          
  Branches        135        135          
==========================================
  Hits           1378       1378          
  Misses          797        797          
  Partials         74         74          

Sunburst

Powered by Codecov. Last update 624fcfa...51a0b65

@jessesquires
Copy link
Owner

Thanks @algrid !

Based on the discussion in #1218 -- I'm not completely sure how a deep copy resolves this. Can you explain more? Do you have before/after repro steps?

@algrid
Copy link
Contributor Author

algrid commented Oct 17, 2016

@jessesquires I can reproduce the original problem in the demo app:

  1. Turn on 'Springy bubbles' in Setting
  2. Push via storyboard
  3. Start scrolling

I get the following in my console:

2016-10-17 22:46:49.101 JSQMessages[5042:411339] Logging only once for UICollectionViewFlowLayout cache mismatched frame
2016-10-17 22:46:49.102 JSQMessages[5042:411339] UICollectionViewFlowLayout has cached frame mismatch for index path <NSIndexPath: 0xc000000004200016> {length = 2, path = 0 - 33} - cached value: {{4, 3421}, {367, 143}}; expected value: {{4, 3423}, {367, 143}}
2016-10-17 22:46:49.102 JSQMessages[5042:411339] This is likely occurring because the flow layout subclass JSQMessagesCollectionViewFlowLayout is modifying attributes returned by UICollectionViewFlowLayout without copying them

With this PR the problem is gone.

I'm doing here exactly what the log message suggests - don't modify attributes returned by UICollectionViewFlowLayout, instead make copies. Inspired by this: http://stackoverflow.com/a/33172179

@jessesquires jessesquires self-assigned this Oct 18, 2016
@jessesquires jessesquires added this to the 8.0.0 milestone Oct 18, 2016
@jessesquires jessesquires merged commit cebe876 into jessesquires:develop Oct 18, 2016
@jessesquires
Copy link
Owner

Thanks @algrid ! 💯

@jessesquires jessesquires removed their assignment Oct 18, 2016
Luke47 pushed a commit to ubergrape/JSQMessagesViewController that referenced this pull request Jan 30, 2017
tom1199 added a commit to tom1199/JSQMessagesViewController that referenced this pull request Apr 6, 2017
pcoltau added a commit to TeletronicsDotAe/JSQMessagesViewController that referenced this pull request Apr 16, 2017
…ViewController into develop

* 'develop' of https://github.com/jessesquires/JSQMessagesViewController: (200 commits)
  fix documentation (jessesquires#2037)
  Added Ginger.io to apps using this library (jessesquires#2017)
  Adding LŌC to list of apps using this library (jessesquires#2004)
  Update apps_using_this_library.md (jessesquires#1989)
  Added support to set placeholder insets (jessesquires#1908). Close jessesquires#1907
  ImageOptim on assets (jessesquires#1845)
  Update PULL_REQUEST_TEMPLATE.md
  Update ISSUE_TEMPLATE.md
  Update .travis.yml (jessesquires#1937)
  Update .travis.yml
  Add Bosnian and  Croatian localization (jessesquires#1928)
  Added app StudyBuddy (jessesquires#1920)
  Added Creds to Core team (jessesquires#1905)
  Add Danish translation of JSQMessages.strings (jessesquires#1898)
  Update apps_using_this_library.md (jessesquires#1896)
  Update .travis.yml to run XCode8 (jessesquires#1872)
  Copy attributes before modifying in layout (jessesquires#1868). Close jessesquires#1218
  Update ISSUE_TEMPLATE.md
  fix warnings/errors
  cleanup from PR jessesquires#1823
  ...
pietrorea pushed a commit to upsidetravel/JSQMessagesViewController that referenced this pull request Jul 24, 2017
FWAlex added a commit to FWAlex/JSQMessagesViewController that referenced this pull request Sep 29, 2017
* develop: (120 commits)
  Update podspecs
  Update README.md
  Add czech localization. (jessesquires#2130)
  fix documentation (jessesquires#2037)
  Added Ginger.io to apps using this library (jessesquires#2017)
  Adding LŌC to list of apps using this library (jessesquires#2004)
  Update apps_using_this_library.md (jessesquires#1989)
  Added support to set placeholder insets (jessesquires#1908). Close jessesquires#1907
  ImageOptim on assets (jessesquires#1845)
  Update PULL_REQUEST_TEMPLATE.md
  Update ISSUE_TEMPLATE.md
  Update .travis.yml (jessesquires#1937)
  Update .travis.yml
  Add Bosnian and  Croatian localization (jessesquires#1928)
  Added app StudyBuddy (jessesquires#1920)
  Added Creds to Core team (jessesquires#1905)
  Add Danish translation of JSQMessages.strings (jessesquires#1898)
  Update apps_using_this_library.md (jessesquires#1896)
  Update .travis.yml to run XCode8 (jessesquires#1872)
  Copy attributes before modifying in layout (jessesquires#1868). Close jessesquires#1218
  ...

# Conflicts:
#	CHANGELOG.md
#	JSQMessagesViewController.podspec
mbesnili pushed a commit to mbesnili/JSQMessagesViewController that referenced this pull request Oct 20, 2017
* develop: (102 commits)
  Update .travis.yml to run XCode8 (jessesquires#1872)
  Copy attributes before modifying in layout (jessesquires#1868). Close jessesquires#1218
  Update ISSUE_TEMPLATE.md
  fix warnings/errors
  cleanup from PR jessesquires#1823
  fix a bug on demo that can't copy successfully. (jessesquires#1871)
  Finished video thumbnail feature (jessesquires#1823)
  fix footer for header in swift example (jessesquires#1833)
  Fix for preloading textview and resizing it (jessesquires#1865) close jessesquires#1709
  added swedish localization (jessesquires#1847)
  Update Swift example to Xcode 8 GM Swift 3.0 syntax (jessesquires#1825)
  empty icons
  cherry pick d6cdf18 by @jstart from master
  link to detailed instructions for custom cells (jessesquires#1828)
  close jessesquires#1768. Assign each UIView element an identifier to make it visible under UI-Testing (jessesquires#1775)
  Input bar support dynamic type (jessesquires#1793)
  Update CHANGELOG.md
  cleanup
  xcode8 beta4 updates
  Changed sendButtonOnRight to sendButtonLocation (jessesquires#840)
  ...

# Conflicts:
#	JSQMessages.xcodeproj/project.pbxproj
#	JSQMessagesDemo/DemoMessagesViewController.m
#	JSQMessagesViewController/Views/JSQMessagesComposerTextView.h
#	Podfile
#	Podfile.lock
#	Pods/Manifest.lock
#	Pods/Pods.xcodeproj/project.pbxproj
#	Pods/Target Support Files/JSQSystemSoundPlayer/JSQSystemSoundPlayer.xcconfig
#	Pods/Target Support Files/Pods-JSQMessages/Pods-JSQMessages-acknowledgements.markdown
#	Pods/Target Support Files/Pods-JSQMessages/Pods-JSQMessages-acknowledgements.plist
#	Pods/Target Support Files/Pods-JSQMessages/Pods-JSQMessages-frameworks.sh
#	Pods/Target Support Files/Pods-JSQMessages/Pods-JSQMessages.debug.xcconfig
#	Pods/Target Support Files/Pods-JSQMessages/Pods-JSQMessages.release.xcconfig
#	Pods/Target Support Files/Pods-JSQMessagesTests/Pods-JSQMessagesTests-resources.sh
#	Pods/Target Support Files/Pods-JSQMessagesTests/Pods-JSQMessagesTests.debug.xcconfig
#	Pods/Target Support Files/Pods-JSQMessagesTests/Pods-JSQMessagesTests.release.xcconfig
#	Pods/Target Support Files/Pods/Pods-acknowledgements.markdown
#	Pods/Target Support Files/Pods/Pods-acknowledgements.plist
#	Pods/Target Support Files/Pods/Pods-frameworks.sh
#	SwiftExample/Pods/Target Support Files/Pods-SwiftExample/Pods-SwiftExample-acknowledgements.markdown
#	SwiftExample/Pods/Target Support Files/Pods-SwiftExample/Pods-SwiftExample-acknowledgements.plist
#	SwiftExample/Pods/Target Support Files/Pods-SwiftExample/Pods-SwiftExample-frameworks.sh
#	SwiftExample/Pods/Target Support Files/Pods-SwiftExampleTests/Pods-SwiftExampleTests-acknowledgements.markdown
#	SwiftExample/Pods/Target Support Files/Pods-SwiftExampleTests/Pods-SwiftExampleTests-acknowledgements.plist
#	SwiftExample/Pods/Target Support Files/Pods-SwiftExampleTests/Pods-SwiftExampleTests-frameworks.sh
N-Pex pushed a commit to N-Pex/JSQMessagesViewController that referenced this pull request Jan 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash while scrolling
3 participants