Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: improve obscure rectangle fit/size #2236

Merged
merged 4 commits into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

### Features

- Session replay Alpha for Android and iOS ([#2208](https://github.com/getsentry/sentry-dart/pull/2208), [#2269](https://github.com/getsentry/sentry-dart/pull/2269)).
- Session replay Alpha for Android and iOS ([#2208](https://github.com/getsentry/sentry-dart/pull/2208), [#2269](https://github.com/getsentry/sentry-dart/pull/2269), [#2236](https://github.com/getsentry/sentry-dart/pull/2236)).

To try out replay, you can set following options (access is limited to early access orgs on Sentry. If you're interested, [sign up for the waitlist](https://sentry.io/lp/mobile-replay-beta/)):

Expand All @@ -27,6 +27,7 @@
...
options.allowUrls = ["^https://sentry.com.*\$", "my-custom-domain"];
options.denyUrls = ["^.*ends-with-this\$", "denied-url"];
options.denyUrls = ["^.*ends-with-this\$", "denied-url"];
},
appRunner: () => runApp(MyApp()),
);
Expand Down
40 changes: 26 additions & 14 deletions flutter/lib/src/replay/widget_filter.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import 'package:flutter/rendering.dart';
import 'package:flutter/services.dart';
import 'package:flutter/widgets.dart';
import 'package:meta/meta.dart';
Expand Down Expand Up @@ -77,25 +78,25 @@

final renderObject = element.renderObject;
if (renderObject is! RenderBox) {
_cantObscure(widget, "it's renderObject is not a RenderBox");
_cantObscure(widget, "its renderObject is not a RenderBox");

Check warning on line 81 in flutter/lib/src/replay/widget_filter.dart

View check run for this annotation

Codecov / codecov/patch

flutter/lib/src/replay/widget_filter.dart#L81

Added line #L81 was not covered by tests
return false;
}

final size = element.size;
if (size == null) {
_cantObscure(widget, "it's renderObject has a null size");
return false;
var rect = _boundingBox(renderObject);

// If it's a clipped render object, use parent's offset and size.
// This helps with text fields which often have oversized render objects.
if (renderObject.parent is RenderStack) {
final renderStack = (renderObject.parent as RenderStack);
final clipBehavior = renderStack.clipBehavior;
if (clipBehavior == Clip.hardEdge ||
clipBehavior == Clip.antiAlias ||
clipBehavior == Clip.antiAliasWithSaveLayer) {

Check warning on line 94 in flutter/lib/src/replay/widget_filter.dart

View check run for this annotation

Codecov / codecov/patch

flutter/lib/src/replay/widget_filter.dart#L93-L94

Added lines #L93 - L94 were not covered by tests
final clipRect = _boundingBox(renderStack);
rect = rect.intersect(clipRect);
Comment on lines +90 to +96
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice 👍 , do we need tests for this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added tests

}
}

final offset = renderObject.localToGlobal(Offset.zero);

final rect = Rect.fromLTWH(
offset.dx * _pixelRatio,
offset.dy * _pixelRatio,
size.width * _pixelRatio,
size.height * _pixelRatio,
);

if (!rect.overlaps(_bounds)) {
assert(() {
logger(SentryLevel.debug, "WidgetFilter skipping offscreen: $widget");
Expand Down Expand Up @@ -151,6 +152,17 @@
"WidgetFilter cannot obscure widget $widget: $message");
}
}

@pragma('vm:prefer-inline')
Rect _boundingBox(RenderBox box) {
final offset = box.localToGlobal(Offset.zero);
return Rect.fromLTWH(
offset.dx * _pixelRatio,
offset.dy * _pixelRatio,
box.size.width * _pixelRatio,
box.size.height * _pixelRatio,
);
}
}

class WidgetFilterItem {
Expand Down
24 changes: 23 additions & 1 deletion flutter/test/replay/test_widget.dart
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,22 @@ Future<Element> pumpTestElement(WidgetTester tester,
Opacity(opacity: 0, child: newImage()),
Offstage(offstage: true, child: Text('Offstage text')),
Offstage(offstage: true, child: newImage()),
Text(dummyText),
SizedBox(
width: 100,
height: 20,
child: Stack(children: [
Positioned(
top: 0,
left: 0,
width: 50,
child: Text(dummyText)),
Positioned(
top: 0,
left: 0,
width: 50,
child: newImage(width: 500, height: 500)),
]))
],
),
),
Expand All @@ -55,4 +71,10 @@ final testImageData = Uint8List.fromList([
// This comment prevents dartfmt reformatting this to single-item lines.
]);

Image newImage() => Image.memory(testImageData, width: 1, height: 1);
Image newImage({double width = 1, double height = 1}) => Image.memory(
testImageData,
width: width,
height: height,
);

const dummyText = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit.';
28 changes: 26 additions & 2 deletions flutter/test/replay/widget_filter_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,15 @@ void main() async {
rootAssetBundle: rootBundle,
);

boundsRect(WidgetFilterItem item) =>
'${item.bounds.width.floor()}x${item.bounds.height.floor()}';

group('redact text', () {
testWidgets('redacts the correct number of elements', (tester) async {
final sut = createSut(redactText: true);
final element = await pumpTestElement(tester);
sut.obscure(element, 1.0, defaultBounds);
expect(sut.items.length, 2);
expect(sut.items.length, 4);
});

testWidgets('does not redact text when disabled', (tester) async {
Expand All @@ -43,14 +46,25 @@ void main() async {
sut.obscure(element, 1.0, Rect.fromLTRB(0, 0, 100, 100));
expect(sut.items.length, 1);
});

testWidgets('correctly determines sizes', (tester) async {
final sut = createSut(redactText: true);
final element = await pumpTestElement(tester);
sut.obscure(element, 1.0, defaultBounds);
expect(sut.items.length, 4);
expect(boundsRect(sut.items[0]), '624x48');
expect(boundsRect(sut.items[1]), '169x20');
expect(boundsRect(sut.items[2]), '800x192');
expect(boundsRect(sut.items[3]), '50x20');
});
});

group('redact images', () {
testWidgets('redacts the correct number of elements', (tester) async {
final sut = createSut(redactImages: true);
final element = await pumpTestElement(tester);
sut.obscure(element, 1.0, defaultBounds);
expect(sut.items.length, 2);
expect(sut.items.length, 3);
});

// Note: we cannot currently test actual asset images without either:
Expand Down Expand Up @@ -93,6 +107,16 @@ void main() async {
sut.obscure(element, 1.0, Rect.fromLTRB(0, 0, 500, 100));
expect(sut.items.length, 1);
});

testWidgets('correctly determines sizes', (tester) async {
final sut = createSut(redactImages: true);
final element = await pumpTestElement(tester);
sut.obscure(element, 1.0, defaultBounds);
expect(sut.items.length, 3);
expect(boundsRect(sut.items[0]), '1x1');
expect(boundsRect(sut.items[1]), '1x1');
expect(boundsRect(sut.items[2]), '50x20');
});
});
}

Expand Down
Loading