Skip to content

Commit

Permalink
[ReactNative] Fix Text Updating Crash
Browse files Browse the repository at this point in the history
Summary:
@public

{D1953613} added an optimization that allowed for shadow nodes that are not
backed by views, but didn't actually work robustly in the remove case because
the indices can get out of sync.  That diff also started returning nil for raw
text nodes, which triggered this bug and broke "see more" functionality in the
`FBTextWithEntities` and `ExpandingText` components, leading to crashes in the
Groups app.

This diff fixes the issue by simply returning `UIView` placeholders again.
Slight perf/ memory cost but no more crashes and there should be no other
adverse affects.

We'll need to think up something more clever in order to properly support `nil`
views in the future, probably something that uses the shadow hierarchy to build
the View hierarchy, rather than mirroring identical commands to both - see
facebook#1102.

Test Plan:
- TextUpdateTest fails without native changes, now passes with them.
- ExpandingText example no longer crashes.
- See More in Groups app no longer crashes.
  • Loading branch information
sahrens committed May 2, 2015
1 parent a4616bf commit 5e110d2
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 5 deletions.
2 changes: 1 addition & 1 deletion Libraries/Text/RCTRawTextManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ @implementation RCTRawTextManager

- (UIView *)view
{
return nil;
return [[UIView alloc] init]; // TODO(#1102) Remove useless views.
}

- (RCTShadowView *)shadowView
Expand Down
5 changes: 1 addition & 4 deletions Libraries/Text/TextUpdateTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ var {
Text,
} = React;

var MIX_TYPES = false; // TODO(#6916648): fix bug and set true

var TestManager = NativeModules.TestManager || NativeModules.SnapshotTestManager;

var TextUpdateTest = React.createClass({
Expand All @@ -42,13 +40,12 @@ var TextUpdateTest = React.createClass({
);
},
render: function() {
var extraText = MIX_TYPES ? 'raw text' : <Text>wrapped text</Text>;
return (
<Text
style={styles.container}
onPress={() => this.setState({seeMore: !this.state.seeMore})}>
<Text>Tap to see more (bugs)...</Text>
{this.state.seeMore && extraText}
{this.state.seeMore && 'raw text'}
</Text>
);
},
Expand Down

0 comments on commit 5e110d2

Please sign in to comment.