After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 763907 - GDK W32: Clipboard handling is not tight enough
GDK W32: Clipboard handling is not tight enough
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: Win32
unspecified
Other All
: Normal normal
: ---
Assigned To: gtk-win32 maintainers
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2016-03-19 09:58 UTC by LRN
Modified: 2016-03-26 00:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GDK W32: Optimize clipboard handling a bit (5.08 KB, patch)
2016-03-19 09:58 UTC, LRN
none Details | Review
GDK W32: Optimize clipboard handling a bit (5.29 KB, patch)
2016-03-19 10:24 UTC, LRN
none Details | Review
GDK W32: Optimize clipboard handling a bit (5.09 KB, patch)
2016-03-25 22:34 UTC, LRN
committed Details | Review

Description LRN 2016-03-19 09:58:29 UTC
OpenClipboard() is called too early, or without need. CloseClipboard()
is not called soon enough.
Comment 1 LRN 2016-03-19 09:58:35 UTC
Created attachment 324322 [details] [review]
GDK W32: Optimize clipboard handling a bit

Delay as long as possible before calling OpenClipboard(),
call CloseClipboard() as quickly as possible after that.
Don't call OpenClipboard() when we don't need to (for example,
we don't need to open clipboard to call GetClipboardOwner()).

Also, print out actual W32 error code in some cases where it
was not printed before.
Comment 2 Ignacio Casal Quinteiro (nacho) 2016-03-19 10:14:31 UTC
Review of attachment 324322 [details] [review]:

Patch looks good to me but fix the nitpicks. Feel free to push with those minor fixes.

::: gdk/win32/gdkdisplay-win32.c
@@ +383,2 @@
         HWND hwndOwner;
+        HWND hwndOpener;

I know it was like that but change it to: hwnd_owner, hwnd_opener

@@ +407,3 @@
+        success = OpenClipboard (hwnd);
+
+        if (!success)

just reomve the success var and put the call to OpenClipboard here
Comment 3 LRN 2016-03-19 10:24:00 UTC
Created attachment 324329 [details] [review]
GDK W32: Optimize clipboard handling a bit

v2:
* Fix nitpicks
* Put all debug stuff under one ifdef, including Open/Close clipboard calls.
Comment 4 Ignacio Casal Quinteiro (nacho) 2016-03-19 10:36:42 UTC
Review of attachment 324329 [details] [review]:

See another nitpick that I missed before.

::: gdk/win32/gdkdisplay-win32.c
@@ +416,3 @@
+                DWORD w32_error = GetLastError ();
+
+                g_warning ("Failed to OpenClipboard on window handle %p: %lu ", hwnd, w32_error);

I would change this to:
Fail on OpenClipboard on window handle...
Also instead of printing the error code print the error message?
Comment 5 LRN 2016-03-19 10:59:14 UTC
(In reply to Ignacio Casal Quinteiro (nacho) from comment #4)
> Review of attachment 324329 [details] [review] [review]:
> 
> I would change this to:
> Fail on OpenClipboard on window handle...

Okay. Anything else?

> Also instead of printing the error code print the error message?

Code is better. You can always look up the code. Looking up a message is more difficult. Code *and* message is ok though.
Comment 6 Ignacio Casal Quinteiro (nacho) 2016-03-19 12:14:03 UTC
OK, I think that's all, also code + message is fine for me.
Comment 7 Fan, Chun-wei 2016-03-19 14:43:26 UTC
Hi,

I think it's also good for me.

With blessings, thank you!
Comment 8 LRN 2016-03-25 22:34:14 UTC
Created attachment 324774 [details] [review]
GDK W32: Optimize clipboard handling a bit

v3:
* Refactored the code a bit, now that W32_API_FAILED() prints
  the error code
Comment 9 Ignacio Casal Quinteiro (nacho) 2016-03-25 22:36:59 UTC
Review of attachment 324774 [details] [review]:

Looks good.
Comment 10 LRN 2016-03-26 00:07:46 UTC
Attachment 324774 [details] pushed as b9b67e0 - GDK W32: Optimize clipboard handling a bit