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

'Copy' from log view copies all previously copied messages #4466

Closed
w1282 opened this issue Jul 7, 2023 · 7 comments
Closed

'Copy' from log view copies all previously copied messages #4466

w1282 opened this issue Jul 7, 2023 · 7 comments
Assignees
Labels
Component: UI Issue needs changes to the user interface Effort: Low Issue should take < 1 week Impact: Medium Issue is impactful with a bad, or no, workaround Type: Bug Issue is a non-crashing bug with repro steps UI: Logs Issues with the Log area
Milestone

Comments

@w1282
Copy link

w1282 commented Jul 7, 2023

Version and Platform (required):

  • Binary Ninja Version: 3.5.4375-dev
  • OS: Windows
  • OS Version: 11
  • CPU Architecture: x64

Bug Description:
'Copy' on a single line from the Log view works as expected the first time it is triggered in a session, but subsequent copies of new messages cause a buffer of all previously copied messages to be constructed. The only way I've identified to fix this is to reload binary ninja entirely.

Steps To Reproduce:
Please provide all steps required to reproduce the behavior:

  1. Go to Log
  2. Right-click and copy a message
  3. Paste it into the Search log filter and observe it works as expected
  4. Delete the content from the 'Search log' box
  5. Go back to Log and right-click/copy a new message
  6. Paste it into the Search log filter and observe both messages were placed into the clipboard

Note that log message are inserted into the clipboard in the order they appear in the Log window, and not the order they were selected for copying.

Expected Behavior:
The clipboard should contain only the message that was selected for copying

@psifertex
Copy link
Member

I can't reproduce this on latest dev on either MacOS or Windows. Any chance you can record a video or confirm you have no other plugins enabled or clipboard utilities that might be impacting it? Is analysis running while this happens? Does it happen on multiple binaries?

@psifertex
Copy link
Member

Nevermind, we've been able to reproduce it intermittently. Thanks!

@bpotchik
Copy link
Member

bpotchik commented Jul 7, 2023

The problem here is that when switching tabs (and BinaryViews), and/or session IDs, the underlying model changes. However, during this switch the selection model is not updated and becomes stale. The stale selection model contains a set of selected log entries which is not representative of what is visually shown.

In order to fix this issue, we need to update/clear/maintain the selection model when the underlying filtering changes due to switching tabs and/or changing sessions.

@bpotchik bpotchik added Type: Bug Issue is a non-crashing bug with repro steps Impact: Medium Issue is impactful with a bad, or no, workaround Effort: Low Issue should take < 1 week labels Jul 7, 2023
@w1282
Copy link
Author

w1282 commented Jul 7, 2023

While I'm not confident on all of the terminology, I can consistently trigger this without any BNDB open and with no plugins loaded, just from the 'New Tab' landing page.

Although I can confirm that it's related to filtering. I hadn't realized but I had only really been testing it by pasting the clipboard into the 'Search log' filter line. Pasting out to a new application does not cause this issue, but as I noted above it can be replicated in just a single window with no BNDB's loaded.

@psifertex
Copy link
Member

Yeah, we think we've identified the bug, the fix is non-trivial right now, but shouldn't be too hard.

Brian left the notes above based on the initial investigation he and I did for whomever ends up taking a look later on our team since we didn't have time to resolve it yet.

@fuzyll fuzyll added Component: UI Issue needs changes to the user interface UI: Logs Issues with the Log area labels Jul 12, 2023
@xusheng6
Copy link
Member

This can also be reproduced by the following steps:

  1. Select any line (lets called it line 1) in the log view
  2. Do a search in the filter, and make sure the line 1 does not match the filter
  3. Clear the search filter
  4. Notice line 1 is no longer selected
  5. Select another line (line 2), right-click, copy
  6. Observe the text of both lines are copied into the buffer

The user's repro step is actually the same as the above. The difference is that he always pastes the text of line 1 into the search filter. And for some reason, we are adding an space to the text of the line when it gets pasted, so line 1 will not match the search criteria.

I think the problem is that 1. the widget should NOT de-select the line if it does not match the search filter, 2. even if it does de-select it, it should send a selectionChanged signal when it does so. Right now, I observe that such signal is NOT sent when the widget de-selects the line, causing our internal tracking of selection status to de-rail.

This could be a Qt bug if we do not do anything obviously wrong that causes the current behavior.

@xusheng6 xusheng6 self-assigned this Aug 10, 2023
@bpotchik bpotchik added this to the Coruscant milestone Aug 29, 2023
@bpotchik
Copy link
Member

bpotchik commented Aug 29, 2023

Fixed in 3.5.4494.

Thanks @xusheng6!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: UI Issue needs changes to the user interface Effort: Low Issue should take < 1 week Impact: Medium Issue is impactful with a bad, or no, workaround Type: Bug Issue is a non-crashing bug with repro steps UI: Logs Issues with the Log area
Projects
None yet
Development

No branches or pull requests

5 participants