SUPERCILEX / gnome-clipboard-history

Gnome Clipboard History is a clipboard manager Gnome extension that saves what you've copied into an easily accessible, searchable history panel.
https://extensions.gnome.org/extension/4839/clipboard-history/
MIT License
455 stars 45 forks source link

Clearing history causes next copy to fail #93

Closed benbaker76 closed 2 years ago

benbaker76 commented 2 years ago

Describe the bug

When a clear history is performed the next copy will fail due to _debouncing++.

How To Reproduce

  1. Click the trash icon to clear history
  2. Select text and copy it

Versions

GNOME Shell 42.2
GCH   Version: 17
org.gnome.shell.extensions.clipboard-history cache-only-favorites false
org.gnome.shell.extensions.clipboard-history cache-size 100
org.gnome.shell.extensions.clipboard-history clear-history @as []
org.gnome.shell.extensions.clipboard-history confirm-clear true
org.gnome.shell.extensions.clipboard-history disable-down-arrow true
org.gnome.shell.extensions.clipboard-history display-mode 0
org.gnome.shell.extensions.clipboard-history enable-keybindings true
org.gnome.shell.extensions.clipboard-history history-size 1000
org.gnome.shell.extensions.clipboard-history move-item-first true
org.gnome.shell.extensions.clipboard-history next-entry @as []
org.gnome.shell.extensions.clipboard-history notify-on-copy false
org.gnome.shell.extensions.clipboard-history paste-on-selection true
org.gnome.shell.extensions.clipboard-history prev-entry @as []
org.gnome.shell.extensions.clipboard-history private-mode false
org.gnome.shell.extensions.clipboard-history process-primary-selection false
org.gnome.shell.extensions.clipboard-history strip-text false
org.gnome.shell.extensions.clipboard-history toggle-menu ['<Super><Shift>V']
org.gnome.shell.extensions.clipboard-history toggle-private-mode ['<Super><Shift>P']
org.gnome.shell.extensions.clipboard-history topbar-preview-size 10
org.gnome.shell.extensions.clipboard-history window-width-percentage 33

Additional context (if a crash, provide stack trace)

When _clearHistory() is called it calls _resetSelectedMenuItem() which calls _setClipboardText('') which in turn increments _debouncing and causes the next copy to fail.

A possible solution is to not increment _debouncing if the text is empty:

  _setClipboardText(text) {
    if (text !== '' && this._debouncing !== undefined) {
      this._debouncing++;
    }

    Clipboard.set_text(St.ClipboardType.CLIPBOARD, text);
    Clipboard.set_text(St.ClipboardType.PRIMARY, text);
  }
SUPERCILEX commented 2 years ago

Hmmm, I wasn't able to repro. For me setting text = '' still results in a _processClipboardContent call. I also tried clearing my history and was able to copy something immediately afterwards and have it show up.

benbaker76 commented 2 years ago

When you clear history is this._debouncing++ in _setClipboardText being executed for you?

For me it happens consistently and then causes this check to return for the next copy:

  _processClipboardContent(text) {
    if (this._debouncing > 0) {
      this._debouncing--;
      return;
    }

I've updated my first post with my setting and I use the paste on selection feature for this issue.

SUPERCILEX commented 2 years ago

Those settings look like mine. For me, this._debouncing--; gets called when I clear history. Before the if statement, you could add log(Me.uuid, "_processClipboardContent", this._debouncing); and then look at the journal: journalctl -f /usr/bin/gnome-shell to see when it shows up.

benbaker76 commented 2 years ago

So I added some log output as requested:

  _clearHistory() {
    log(Me.uuid, "_clearHistory _debouncing:", this._debouncing);
    log(Me.uuid, "this.currentlySelectedEntry:", this.currentlySelectedEntry);
    log(Me.uuid, "this.currentlySelectedEntry.favorite:", this.currentlySelectedEntry.favorite);
    if (this.currentlySelectedEntry && !this.currentlySelectedEntry.favorite) {
      log(Me.uuid, "_resetSelectedMenuItem");
      this._resetSelectedMenuItem();
    }

  _setClipboardText(text) {
    log(Me.uuid, "_setClipboardText _debouncing:", this._debouncing);
    if (this._debouncing !== undefined) {
      this._debouncing++;
      log(Me.uuid, "_setClipboardText _debouncing++:", this._debouncing);
    }

  _processClipboardContent(text, html) {
    log(Me.uuid, "_processClipboardContent _debouncing:", this._debouncing);
    if (this._debouncing > 0) {
      this._debouncing--;
      log(Me.uuid, "_processClipboardContent _debouncing--:", this._debouncing);
      return;
    }

Here is the output I get:

# Clear history
clipboard-history@alexsaveau.dev, _clearHistory _debouncing:, 0
clipboard-history@alexsaveau.dev, this.currentlySelectedEntry:, [object Object]
clipboard-history@alexsaveau.dev, this.currentlySelectedEntry.favorite:, false
clipboard-history@alexsaveau.dev, _resetSelectedMenuItem
clipboard-history@alexsaveau.dev, _setClipboardText _debouncing:, 0
clipboard-history@alexsaveau.dev, _setClipboardText _debouncing++:, 1
# Copy text
clipboard-history@alexsaveau.dev, _processClipboardContent _debouncing:, 1
clipboard-history@alexsaveau.dev, _processClipboardContent _debouncing--:, 0

I'm not sure how this._debouncing--; is getting called when you clear history? As far as I can tell processClipboardContent is never called from _clearHistory.

SUPERCILEX commented 2 years ago

I get this:

# Clear history
clipboard-history@alexsaveau.dev, _clearHistory _debouncing:, 0
clipboard-history@alexsaveau.dev, _setClipboardText _debouncing:, 0
clipboard-history@alexsaveau.dev, _processClipboardContent _debouncing:, 1
# Copy text
clipboard-history@alexsaveau.dev, _processClipboardContent _debouncing:, 0

No idea why they're different.

Maybe a different approach to debouncing is to unregister the clipboard change listener before writing to it and then re-register it.

benbaker76 commented 2 years ago

I just tried a clean install and it works fine now. It must have been from a modification I made. My apologies for wasting your time!

SUPERCILEX commented 2 years ago

No worries!