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

NativeClipboard.SetData related API takes no effect #355

Closed
zhuxb711 opened this issue Dec 19, 2022 · 47 comments
Closed

NativeClipboard.SetData related API takes no effect #355

zhuxb711 opened this issue Dec 19, 2022 · 47 comments

Comments

@zhuxb711
Copy link

Describe the bug and how to reproduce
Just like the code what I provided below

What code is involved

using (NativeClipboard Clipboard = new NativeClipboard(true))
{
       Clipboard.SetText("12333333");
}

//Check system clipboard: No content like "12333333"

Expected behavior
Looks like the content did not write down to clipboard

@dahall
Copy link
Owner

dahall commented Dec 20, 2022

I just ran this test without error:

const string txt = @"“We’ve been here”";
using (var cb = new Clipboard())
   cb.SetText(txt, $"<p>{txt}</p>");
using (var cb = new Clipboard())
{
   Assert.That(cb.GetText(TextDataFormat.UnicodeText), Is.EqualTo(txt));
   Assert.That(cb.GetText(TextDataFormat.Html), Contains.Substring(txt));
}

@dahall
Copy link
Owner

dahall commented Dec 20, 2022

FYI: The Dispose method actually closes the clipboard to publish the changes. I don't love the design, but it was the best I could imagine.

@zhuxb711
Copy link
Author

Hi @dahall I ran your code but different from my side.

            const string txt = @"“We’ve been here”";
            using (var cb = new NativeClipboard())
                cb.SetText(txt, $"<p>{txt}</p>");
            using (var cb = new NativeClipboard())
            {
                System.Diagnostics.Debug.WriteLine($"What stored in clipboard: {cb.GetText(TextDataFormat.UnicodeText)}, what we suppose to {txt}, they are equal? {(cb.GetText(TextDataFormat.UnicodeText) == txt ? "Yes" : "No")}");
                System.Diagnostics.Debug.WriteLine($"What stored in clipboard: {cb.GetText(TextDataFormat.UnicodeText)}, what we suppose to {txt}, they are equal? {(cb.GetText(TextDataFormat.Html).Contains(txt) ? "Yes" : "No")}");
            }

I got this:

What stored in clipboard: , what we suppose to “We’ve been here”, they are equal? No
System.InvalidOperationException:“HTML format header cannot be processed.”

My test env is in WinAppSdk 1.2, STA thread

@dahall
Copy link
Owner

dahall commented Dec 23, 2022

Would you mind testing the following code on your system and also let me know your platform (Any CPU, x64, etc) of the app consuming the library?

const string txt = @"“We’ve been here”";
using (var cb = new NativeClipboard())
   cb.SetText(txt, TextDataFormat.UnicodeText);
using (var cb = new NativeClipboard())
   System.Diagnostics.Debug.WriteLine($"What stored in clipboard: {cb.GetText(TextDataFormat.UnicodeText)}, what we suppose to {txt}, they are equal? {(cb.GetText(TextDataFormat.UnicodeText) == txt ? "Yes" : "No")}");
const string html = "<p>Body text</p>";
using (var cb = new NativeClipboard())
   cb.SetText(html, TextDataFormat.Html);
using (var cb = new NativeClipboard())
   System.Diagnostics.Debug.WriteLine($"What stored in clipboard: {cb.GetText(TextDataFormat.Html)}, what we suppose to {html}, they are equal? {(cb.GetText(TextDataFormat.Html) == html ? "Yes" : "No")}");

@dahall
Copy link
Owner

dahall commented Dec 23, 2022

I can't get mine to fail so the request above is to help narrow my testing. SetText(string,string,string) calls SetText(string,TextDataFormat).

@zhuxb711
Copy link
Author

First System.Diagnostics.Debug.WriteLine shows:

What stored in clipboard: , what we suppose to “We’ve been here”, they are equal? No

And Second System.Diagnostics.Debug.WriteLine:

System.InvalidOperationException

@zhuxb711
Copy link
Author

But this time we could found the “We’ve been here” in the clipboard history
image

@dahall dahall closed this as completed in 0f00ef4 Dec 26, 2022
@dahall
Copy link
Owner

dahall commented Dec 26, 2022

This is now fixed. It is important to use the first parameter in the NativeClipboard ctor set to true to clear the clipboard before setting values.

The following should now all work with the packages now available on AppVeyor (see homepage):

const string txt = @"“We’ve been here”";
using (var cb = new NativeClipboard(true))
    cb.SetText(txt, $"<p>{txt}</p>");
using (var cb = new NativeClipboard())
{
    System.Diagnostics.Debug.WriteLine($"What stored in clipboard: {cb.GetText(TextDataFormat.UnicodeText)}, what we suppose to {txt}, they are equal? {(cb.GetText(TextDataFormat.UnicodeText) == txt ? "Yes" : "No")}");
    System.Diagnostics.Debug.WriteLine($"What stored in clipboard: {cb.GetText(TextDataFormat.UnicodeText)}, what we suppose to {txt}, they are equal? {(cb.GetText(TextDataFormat.Html).Contains(txt) ? "Yes" : "No")}");
}
using (var cb = new NativeClipboard(true))
   cb.SetText(txt, TextDataFormat.UnicodeText);
using (var cb = new NativeClipboard())
   System.Diagnostics.Debug.WriteLine($"What stored in clipboard: {cb.GetText(TextDataFormat.UnicodeText)}, what we suppose to {txt}, they are equal? {(cb.GetText(TextDataFormat.UnicodeText) == txt ? "Yes" : "No")}");
const string html = "<p>Body text</p>";
using (var cb = new NativeClipboard(true))
   cb.SetText(html, TextDataFormat.Html);
using (var cb = new NativeClipboard())
   System.Diagnostics.Debug.WriteLine($"What stored in clipboard: {cb.GetText(TextDataFormat.Html)}, what we suppose to {html}, they are equal? {(cb.GetText(TextDataFormat.Html) == html ? "Yes" : "No")}");

@zhuxb711
Copy link
Author

Yes, it works, but failed if I passed the WindowHandle of my application to the .ctor()

@dahall dahall reopened this Dec 27, 2022
@dahall
Copy link
Owner

dahall commented Dec 27, 2022

Please try the latest AppVeyor packages. I have used every iteration of the constructor without failure. Let me know if you still have errors.

@dahall
Copy link
Owner

dahall commented Dec 30, 2022

@zhuxb711 : I have the libraries tested for the next NuGet package update. I'm just waiting for your confirmation on these NativeClipboard changes. Let me know when you've tested. Thanks!

@zhuxb711
Copy link
Author

Yes I tested it, still not working. If remove the handle of the demo application, then everything works as expected
Demo WinAppSdk: ClipboardTest.zip

@dahall dahall closed this as completed in 7ca924e Dec 30, 2022
@dahall
Copy link
Owner

dahall commented Dec 31, 2022

That took some work, mostly because I missed a disposal problem. All fixed now, even in your test app! Please confirm.

@dahall dahall reopened this Dec 31, 2022
@zhuxb711
Copy link
Author

I still got this after reinstall 3.4.12 from AppVeyor "ExceptionType: InvalidOperationException, Message: HTML format header cannot be processed."

@zhuxb711
Copy link
Author

zhuxb711 commented Jan 7, 2023

I saw you refactor the whole NativeClipboard, could share more information?

@dahall
Copy link
Owner

dahall commented Jan 15, 2023

I saw you refactor the whole NativeClipboard, could share more information?

NativeClipboard was a mess. Given the paradigm I chose, I could not get it to consistently provide error-free execution. I moved, like the .NET team, to an IDataObject based implementation with better quality results. I just pushed a final update with some examples in the documentation for the class. Please give it a try and let me know your findings. Sorry for the work caused by refactoring, but it was the only way to remove all the bugs.

@dahall dahall closed this as completed Jan 15, 2023
@dahall dahall reopened this Jan 15, 2023
@dahall
Copy link
Owner

dahall commented Jan 15, 2023

I just had a thought of a way to retain some of the structure from before. Give me the afternoon to see if I can get it working.

@dahall
Copy link
Owner

dahall commented Jan 15, 2023

After playing with it, I think it is too convoluted. I think I'll stick with the changes as committed. My apologies again for the work this may cause you.

@dahall dahall closed this as completed Jan 15, 2023
@zhuxb711
Copy link
Author

It seems that your latest changes in NativeClipboard is not included in 3.4.12

@dahall
Copy link
Owner

dahall commented Jan 16, 2023

Which changes? It appears to be complete.

@zhuxb711
Copy link
Author

It seems different from my side and your source code.
image
image
image

@zhuxb711
Copy link
Author

@dahall Thanks for your work, I tested 3.4.13, SetShellItems and GetShellItemArray works for me now. But there is an issue that this code do not work for me. Please check it, thanks.

const string txt = @"“We’ve been here”";
NativeClipboard.SetText(txt, $"<p>{txt}</p>");

System.Diagnostics.Debug.WriteLine($"What stored in clipboard: {NativeClipboard.GetText(TextDataFormat.UnicodeText)}, what we suppose to {txt}, they are equal? {(NativeClipboard.GetText(TextDataFormat.UnicodeText) == txt ? "Yes" : "No")}"); // Throw "Invalid FORMATETC structure (0x80040064 (DV_E_FORMATETC))" here
System.Diagnostics.Debug.WriteLine($"What stored in clipboard: {NativeClipboard.GetText(TextDataFormat.UnicodeText)}, what we suppose to {txt}, they are equal? {(NativeClipboard.GetText(TextDataFormat.Html).Contains(txt) ? "Yes" : "No")}");

By the way, I want to know should I / how to set my window as the window which opened Clipboard? And should I / how to release the Clipboard so that the other process could use clipboard?. Should I call Flush() to make the clipboard available to others process?

@dahall
Copy link
Owner

dahall commented Jan 24, 2023

But there is an issue that this code do not work for me. Please check it, thanks.

This is now fixed. Minor oversight in my code.

By the way, I want to know should I / how to set my window as the window which opened Clipboard? And should I / how to release the Clipboard so that the other process could use clipboard?. Should I call Flush() to make the clipboard available to others process?

You no longer claim the clipboard for the window. Each method is atomic. Once the method completes, the clipboard belongs again to the system.

@dahall
Copy link
Owner

dahall commented Jan 24, 2023

Please see if 3.4.13 from Appveyor resolves the issue with SetText.

@dahall
Copy link
Owner

dahall commented Jan 24, 2023

I also just pushed a commit that fixed a bug in NativeClipboard.Clear.

@dahall dahall closed this as completed Jan 24, 2023
@zhuxb711
Copy link
Author

Hi @dahall , I think it not a good idea to flush the whole IDataObject to system on each SetData(). Some formats such as CFSTR_FILEDESCRIPTORW and CFSTR_FILECONTENTS, we must use them together, but we could not do that on current mechanism.

So a better solution for NativeClipboard is allow user SetData multiple times to the IDataObject instance and retrieve what they set before from the same instance, only release it to system if user call Flush(). If release the IDataObject to system in each calls, we could not set multiple kinds of data to Clipboard (for example: SetShellItemArray & SetText to clipboard).

Here is a demo for you.
ClipboardTest.zip

@zhuxb711
Copy link
Author

On the other hands, we might also set this data to indicate what we want to perform in the clipboard. It's necessary for user to set multiple data to clipboard at the same time.

//Do copy action
NativeClipboard.SetBinaryData(NativeClipboard.RegisterFormat(Shell32.ShellClipboardFormat.CFSTR_PREFERREDDROPEFFECT), new byte[] { 5, 0, 0, 0 })

@zhuxb711
Copy link
Author

Another bugs, could not retrieve the FileContents set before. Invalid FORMATETC structure was threw in Line 134

//Throw Invalid FORMATETC structure
var result = NativeClipboard.GetData(Shell32.ShellClipboardFormat.CFSTR_FILECONTENTS, DVASPECT.DVASPECT_CONTENT, Index);

Here is the demo.
ClipboardTest.zip

@dahall
Copy link
Owner

dahall commented Jan 27, 2023

we could not set multiple kinds of data to Clipboard (for example: SetShellItemArray & SetText to clipboard).

See example in the docs for NativeClipboard for how to get an IDataObject and set multiple items before committing to the clipboard. This resolves your last two comments. I'll work through the bug this weekend.

@zhuxb711
Copy link
Author

But it's almost useless if NativeClipboard do not support SetData multiple times and we still need to operate the IDataObject directly. At this scenario, only NativeClipboard.CreateEmptyIDataObject and NativeClipboard.SetIDataObject is used.

Some APIs in NativeClipboard will retrieve the current IDataObject and set new data to it (Combination). But the others just replace the current IDataObject with a new one (Replacement). I think all the API should use Combination mode.

Besides if each calls to SetData will release the whole IDataObject to system, then API Flush() is useless.

@dahall dahall reopened this Jan 29, 2023
@dahall
Copy link
Owner

dahall commented Jan 29, 2023

only NativeClipboard.CreateEmptyIDataObject and NativeClipboard.SetIDataObject is used.

You've given me an idea, since this class has been so problematic. I've tried doing what you suggest, but flushing and releasing is inconsistent and problematic. What if I just leave those two methods and have all work done through IDataObject? I can take out all the redundant atomic methods. What are your thoughts?

Maybe something like:

using (NativeClipboard cb = new())
{
  DROPDESCRIPTION dropDesc = new() { type = DROPIMAGETYPE.DROPIMAGE_COPY, szMessage = "Move this" };
  cb.Object.SetData(ShellClipboardFormat.CFSTR_DROPDESCRIPTION, dropDesc);
  FILE_ATTRIBUTES_ARRAY faa = new() { cItems = 1, rgdwFileAttributes = new[] { 4U } };
  cb.Object.SetData(ShellClipboardFormat.CFSTR_FILE_ATTRIBUTES_ARRAY, faa);
  FILEGROUPDESCRIPTOR fgd = new() { cItems = (uint)files.Length, fgd = new FILEDESCRIPTOR[files.Length] };
  for (int i = 0; i < files.Length; i++)
  {
    if (i == 0) { cb.Object.SetData(ShellClipboardFormat.CFSTR_FILENAMEA, files[i]); cb.Object.SetData(ShellClipboardFormat.CFSTR_FILENAMEW, files[i]); }
    fgd.fgd[i] = new FileInfo(files[i]);
    ShlwApi.SHCreateStreamOnFileEx(fgd.fgd[i].cFileName, STGM.STGM_READ | STGM.STGM_SHARE_DENY_WRITE, 0, false, null, out IStream istream).ThrowIfFailed();
    cb.Object.SetData(ShellClipboardFormat.CFSTR_FILECONTENTS, istream, DVASPECT.DVASPECT_CONTENT, i);
  }
  cb.Object.SetData(ShellClipboardFormat.CFSTR_FILEDESCRIPTORW, fgd);
}

@zhuxb711
Copy link
Author

Maybe leave those two methods and move the rest to the extension method to IDataObject.
For example:

IDataObject Object = NativeClipboard.CreateEmptyIDataObject();
Object.SetBinaryData();
Object.SetShellItemArray();
NativeClipboard.SetIDataObject(Object);

@dahall
Copy link
Owner

dahall commented Jan 29, 2023

Actually, I like yours better. It is more intentional. Going that direction now.

@dahall
Copy link
Owner

dahall commented Jan 29, 2023

Here's a problem that exists in this scenario. The internal method that sets shell items actually creates an IDataObject -- it doesn't merely add to one. In that case, you'd have to do something like:

IDataObject Object = new ShellItemArray(shellitemlist).ToDataObject();
Object.SetBinarayData();
NativeClipboard.SetIDataObject(Object);

Is that a problem?

@zhuxb711
Copy link
Author

I want to know if there is a way to set ShellItems through IDataObject.SetData? If that is possible, everything works fine.

If not, maybe provide a new method such as NativeClipboard.CreateIDataObjectFromShellItems instead of an extension method to set the ShellItems. That will force the user to set the ShellItems at the beginning rather than through extension method.

@dahall
Copy link
Owner

dahall commented Jan 29, 2023

I want to know if there is a way to set ShellItems through IDataObject.SetData?

There is, lots of manual work that has proven to be very error prone. That's why I gave up on it and moved to SHCreateDataObject which creates the IDataObject.

provide a new method such as NativeClipboard.CreateIDataObjectFromShellItems instead of an extension method to set the ShellItems.

I like this approach. It keeps things simple.

@zhuxb711
Copy link
Author

It looks great, thanks for your work. The last suggestion is

Design the API like this:
NativeClipboard.CreateIDataObjectFromShellItems(params ShellItem[] Items)

if Items.Length == 0, just call NativeClipboard.CreateEmptyIDataObject()

It makes the user easier to use the API if they want to set ShellItems (even they passed an empty array)

@dahall
Copy link
Owner

dahall commented Jan 29, 2023

Having just adjusted all the unit tests, I will warn you that these changes force lots of refactoring. See unit tests for examples.

dahall added a commit that referenced this issue Jan 29, 2023
… to set data have been removed due to inconsistencies and design challenges. New model forces all setting and getting to be done through IDataObject and it's methods and extensions. See documentation for class for example. Addresses #355.
@dahall
Copy link
Owner

dahall commented Jan 29, 2023

Please test latest release and reply with comments or issues.

@zhuxb711
Copy link
Author

zhuxb711 commented Feb 1, 2023

@dahall Since we have an extension method called IsFormatAvailable() for IDataObject. Should we remove the same one from NativeClipboard.IsFormatAvailable() ?

@zhuxb711
Copy link
Author

zhuxb711 commented Feb 1, 2023

Another bugs, could not retrieve the FileContents set before. Invalid FORMATETC structure was threw in Line 134

//Throw Invalid FORMATETC structure
var result = NativeClipboard.GetData(Shell32.ShellClipboardFormat.CFSTR_FILECONTENTS, DVASPECT.DVASPECT_CONTENT, Index);

Here is the demo. ClipboardTest.zip

This issue is still exists. May you take a look at it?

@dahall
Copy link
Owner

dahall commented Feb 2, 2023

Since we have an extension method called IsFormatAvailable() for IDataObject. Should we remove the same one from NativeClipboard.IsFormatAvailable() ?

The IDataObject one will look at only what is in the object. The NativeClipboard one looks at what is currently in the clipboard.

@dahall
Copy link
Owner

dahall commented Feb 2, 2023

This issue is still exists. May you take a look at it?

Checking it out now

@dahall
Copy link
Owner

dahall commented Feb 3, 2023

So it is curious. If you call this on the IDataObject just after setting it and before pushing it to the clipboard, it works. If you call this on the IDataObject you get from the clipboard after setting it, then it fails. The only explanation is that the clipboard cannot maintain the data format. The only hint I got from reading lots of posts is that the IDataObject returned by SHCreateDataObject does not support multiple streams. That seems incorrect though since it works before pushing to the clipboard. I'm out of ideas. If you can make it work, please post your findings.

@dahall dahall closed this as not planned Won't fix, can't repro, duplicate, stale Feb 3, 2023
@zhuxb711
Copy link
Author

zhuxb711 commented Feb 3, 2023

I do not have ideas too... I could read the FILECONTENTS as IStream created by Windows Explorer.

@dahall
Copy link
Owner

dahall commented Feb 5, 2023

I hate loose ends, so today I wrote the following and confirmed that something is wrong with OleSetClipboard.

HRESULT hr = OleInitialize(NULL);

auto id0 = RegisterClipboardFormat(CFSTR_FILEDESCRIPTORW);
auto id = RegisterClipboardFormat(CFSTR_FILECONTENTS);

IDataObject* pDataObj;
if (SUCCEEDED(hr = SHCreateDataObject(NULL, 0, NULL, NULL, IID_PPV_ARGS(&pDataObj))))
{
	FORMATETC fmt0 = { id0, NULL, DVASPECT::DVASPECT_CONTENT, -1, TYMED_HGLOBAL };
	FILEDESCRIPTORW fd0 = { FD_FILESIZE };
	//fd0.nFileSizeLow = 445;
	lstrcpy(fd0.cFileName, L"C:\\Temp\\test.txt");
	auto hMem = GlobalAlloc(GMEM_MOVEABLE, sizeof(fd0));
	auto ptr = GlobalLock(hMem);
	memcpy(ptr, &fd0, sizeof(fd0));
	GlobalUnlock(hMem);
	STGMEDIUM medium0 = { TYMED_HGLOBAL };
	medium0.hGlobal = hMem;
	hr = pDataObj->SetData(&fmt0, &medium0, TRUE);

	FORMATETC fmt = { id, NULL, DVASPECT::DVASPECT_CONTENT, 0, TYMED_ISTREAM };
	STGMEDIUM medium = { TYMED_ISTREAM };
	IStream* pStream;
	if (SUCCEEDED(hr = SHCreateStreamOnFileW(L"C:\\Temp\\test.txt", STGM_READWRITE, &pStream)))
	{
		medium.pstm = pStream;
		if (SUCCEEDED(hr = pDataObj->SetData(&fmt, &medium, TRUE)))
		{
			hr = OleSetClipboard(pDataObj);
			hr = OleFlushClipboard();
		}
		pStream->Release();
	}
	pDataObj->Release();
	pDataObj = NULL;
}

if (SUCCEEDED(hr = OleGetClipboard(&pDataObj)))
{
	FORMATETC fmt = { id, NULL, DVASPECT::DVASPECT_CONTENT, 0, TYMED_ISTREAM | TYMED_HGLOBAL | TYMED_ISTORAGE };
	hr = pDataObj->QueryGetData(&fmt);
	std::cout << "HR = " << hr << std::endl; // This returns the same FORMATETC error as C#
}

OleUninitialize();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants