Add a [valid] property of [MouseTrackerAnnotation] indicates the annotation states. #69866
Conversation
| if (target is MouseTrackerAnnotation) { | ||
| if (target is RenderObject) { | ||
| // We should ignore the detached renderObject, otherwise, may trigger using the disposed object. | ||
| if (!target.attached) |
There was a problem hiding this comment.
!attached is not quite the same as disposed.
@goderbauer WDYT of this?
There was a problem hiding this comment.
Yes, what you pointed out is correct, the range of detached is greater than and includes disposed, so here we need to consider whether it is reasonable to ignore the detached renderObject mouse event handling
There was a problem hiding this comment.
I think this behavior is reasonable. If an RO has detached from the tree, it should not respond to any mouse events. The particularity of this problem is that the RO was detached after the hittest, so it is reasonable and controllable to ignore the handling of this event here. Additionally, if the RO is retaken later for some reasons, this change will not affect it, because it is a real-time conditional control and does not remove the callback.
There was a problem hiding this comment.
That makes sense to me - we may want to update the comment to explain this rather than refering to disposal, which isn't really a thing for RenderObjects.
I'd also defer to @goderbauer or @dkwingsmt on this - I think they're more familiar with this area.
| if (target is MouseTrackerAnnotation) { | ||
| if (target is RenderObject) { | ||
| // It's possible that the renderObject was detached after hitTest, so we should ignore it. | ||
| // https://github.com/flutter/flutter/issues/67044 |
There was a problem hiding this comment.
Instead of linking to an issue here we should just include the relevant information right here in the comment.
| annotations[entry.target as MouseTrackerAnnotation] = entry.transform!; | ||
| final HitTestTarget target = entry.target; | ||
| if (target is MouseTrackerAnnotation) { | ||
| if (target is RenderObject) { |
There was a problem hiding this comment.
It is odd that we have to special case RenderObjects here. What about other MouseTrackerAnnotations that have become "invalid"?
There was a problem hiding this comment.
Do we have a situation where the MouseTrackerAnnotations is not renderObject?
There was a problem hiding this comment.
We're about to. #68674 is introducing a subclass of TextSpan to be a MouseTrackerAnnotations. Although there is no way for that to be turned off. I'm thinking if we should have a generalized way of turning it off.
There was a problem hiding this comment.
If we introduce a new property of MouseTrackerAnnotation to indicate whether it is valid, does the Span have chance to set it?
There was a problem hiding this comment.
@dkwingsmt Hi, Sorry to bother you again ：）
WDYT of a generalized way of turning it off without breaking any API?
|
If we introduce a new property of |
| @@ -112,6 +116,7 @@ class MouseTrackerAnnotation with Diagnosticable { | |||
| ifEmpty: '<none>', | |||
| )); | |||
| properties.add(DiagnosticsProperty<MouseCursor>('cursor', cursor, defaultValue: MouseCursor.defer)); | |||
| properties.add(DiagnosticsProperty<bool>('valid', valid, defaultValue: true)); | |||
There was a problem hiding this comment.
Make it a FlagProperty and ifFalse: 'invalid for MouseTracker'
There was a problem hiding this comment.
Actually let's not add it here, since MouseTrackerAnnotation always return valid: true. Classes with a mutable valid will always have to implement MouseTrackerAnnotation and have to reimplement this method. Add this line to RenderMouseRegion instead.
| @@ -732,6 +732,9 @@ mixin _PlatformViewGestureMixin on RenderBox implements MouseTrackerAnnotation { | |||
| @override | |||
| MouseCursor get cursor => MouseCursor.uncontrolled; | |||
|
|
|||
| @override | |||
| bool valid = true; | |||
|
@dkwingsmt Hi, |
Description
When the mouse long-press and drag the
Draggablechild will trigger replace the child withchildWhenDragging. If the old child has listened the mouse event will trigger using disposed object when continue to move the mouse.Solution
Add a
validproperty ofMouseTrackerAnnotationto prevent the callbacks from being called whenvalidis false.Set the
validfalse when theRenderMouseRegiondetached and set it true when the RO attached.@dnfield @dkwingsmt Hi, please review, thanks very much.
Related Issues
Fixes #69774
Fixes #67044
Fixes #70295
Tests
I added the following tests:
See files.