-
Notifications
You must be signed in to change notification settings - Fork 972
Put back in handler for null/empty context menu detail. This is requi… #6250
Conversation
@@ -664,6 +664,8 @@ const doAction = (action) => { | |||
if (action.notify) { | |||
ipc.send('autofill-popup-hidden', action.tabId) | |||
} | |||
} else { | |||
windowState = windowState.delete('contextMenuDetail') |
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.
I think we should just move L663
here. Just like reverting the part of this commit 7fb2e4c#diff-733d30fbf34816322d3f25dd59c1eb99L651
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.
@darkdh OK fixed- because I am not sure if the order matters, I moved the delete to above this check
…red in order to hide any ContextMenu object Caused by 7fb2e4c#diff-733d30fbf34816322d3f25dd59c1eb99 Fixes #6236 Auditors: @bridiver, @darkdh Test Plan: 1. Launch Brave and use the kabob / cheeseburger icon 2. Notice the context menu comes up 3. Click anywhere on the screen (outside of the context menu) 4. Notice context menu closes itself
Merging... I can implement any other feedback in a follow up; let me know 😄 |
Did this fix #6253 too? |
@luixxiul it will fix issues with regards to the menu- but I am not sure it fixes the other buttons. I'll update here once I can try |
@luixxiul buttons work OK for me? I'm not sure if it's a result of this or not though |
CC @srirambv |
git rebase -i
to squash commits (if needed).Put back in handler for null/empty context menu detail. This is required in order to hide any ContextMenu object
Caused by 7fb2e4c#diff-733d30fbf34816322d3f25dd59c1eb99
Fixes #6236
Auditors: @bridiver, @darkdh
Test Plan: