Fix a crash when removing persistent widgets #10063
Conversation
When a persistent widget is removed, multiple calls to updateShowWidgetInPip happen in succession as each of the widget stores emit updates. But by depending on this.state.persistentWidgetId at the time of the call rather than passing an update function to setState, this had the effect that the removal of the widget could be reverted in the component's state, and so it could end up passing the ID of a removed widget to WidgetPip.
robintown
added
the
T-Defect
| persistentWidgetId = this.state.persistentWidgetId, | ||
| persistentRoomId = this.state.persistentRoomId, | ||
| ): void { | ||
| private updateShowWidgetInPip(): void { |
There was a problem hiding this comment.
This changed from public to private. Should we keep it public?
t3chguy
added
the
X-Release-Blocker
|
Closing in favour of #10099 which just makes sure we don't make a method private without thinking through the interface change. @robintown please feel free to fix the public/private thing later if you think it's the right change. Also, more importantly, it would be great if we could test this, as Michael mentioned. |
|
OK, thank you for helping get this merged. I left this PR alone because a unit test is insufficient to test for this defect, and I didn't have time this particular week to troubleshoot my Cypress setup and learn how to write Cypress tests. I will put testing for this on my to-do list. |
|
Brilliant, thank you for fixing it @robintown , and thank you in advance for writing the test! |
When a persistent widget is removed, multiple calls to
updateShowWidgetInPiphappen in succession as each of the widget stores emit updates. But by depending onthis.state.persistentWidgetIdat the time of the call rather than passing an update function tosetState, this had the effect that the removal of the widget could be reverted in the component's state, and so it could end up passing the ID of a removed widget toWidgetPip.Closes element-hq/element-web#24412
Here's what your changelog entry will look like:
🐛 Bug Fixes