-
Notifications
You must be signed in to change notification settings - Fork 973
Prevent borkiness when session-store file is missing perWindowState[].closedFrames #13265
Conversation
Codecov Report
@@ Coverage Diff @@
## master #13265 +/- ##
==========================================
+ Coverage 56.01% 56.24% +0.23%
==========================================
Files 282 282
Lines 28211 28221 +10
Branches 4642 4646 +4
==========================================
+ Hits 15801 15873 +72
+ Misses 12410 12348 -62
|
…o perWindowState[].closedFrames property, which would then cause the app never recover, even after restart. Fix #13261, although since that's not reproducible I'm not sure the moving-tab keyboard shortcut neccessarily caused it, but something did, and this will allow recovery in that situation.
52e4baf
to
9fb671b
Compare
…nning filterNot (small code tweak) Auditors: @petemill
- Add unit tests for APP_SHOW_NOTIFICATION and APP_HIDE_NOTIFICATION - deregister mock from previous commit (windowStateTest). Forgot that :) Auditors: @petemill
if (style) { | ||
const styleIndex = notifications.findLastIndex((notification) => { | ||
return notification.get('options').get('style') === style | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added {
so that variables defined here are scoped
|
||
// Insert notification next to those with the same style, or at the end | ||
let insertIndex = notifications.size | ||
const style = action.detail |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated this to handle detail
being falsey
return notification.get('message') === action.message | ||
})) | ||
break | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{
for scope (same as above)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great changes 😄 👍 Added some tests here. Take a peek, let me know what you think!
Prevent borkiness when session-store file is missing perWindowState[].closedFrames
0.22.x 4388e3c |
This would then cause the app never recover, even after restart. Especially with an
about:error
tab in the session-store, which would immediately causeappActions.removeHistorySite
to fire when it fails to load. The reducer of that action relied onwindowState.closedFrames
being there. This changes it to not rely on that.Fix #13261, although since that's not reproducible I'm not sure the moving-tab keyboard shortcut necessarily caused it, but something did, and this will allow recovery in that situation.
Submitter Checklist:
git rebase -i
to squash commits (if needed).Test Plan:
Reviewer Checklist:
Tests