Opened 5 years ago

Closed 5 years ago

#15280 closed defect (fixed)

wxHTMLDataObject not working with wxDataObjectComposite on windows

Reported by: eurecam-benjamin Owned by:
Priority: normal Milestone: 3.2.0
Component: wxMSW Version: stable-latest
Keywords: wxHTMLDataObject wxDataObjectComposite wxDataFormat Cc:
Blocked By: Blocking:
Patch: yes


On windows there is an HtmlFormatFixup function (found in src/msw/ole/dataobj.cpp). However this fixup is missing in wxDataObjectComposite::GetObject function preventing its working.

To fix that, I copied the fixup also to wxDataObjectBase::IsSupported. I'm not sure it is the best place, but it works.

Included a patch and a test program.

Attachments (3)

patch.diff download (1.3 KB) - added by eurecam-benjamin 5 years ago.
Patch for dobjcmn.cpp
test_copy.cpp download (3.0 KB) - added by eurecam-benjamin 5 years ago.
Test program
better_patch.diff download (2.7 KB) - added by eurecam-benjamin 5 years ago.
A new and cleaner version

Download all attachments as: .zip

Change History (7)

Changed 5 years ago by eurecam-benjamin

Patch for dobjcmn.cpp

Changed 5 years ago by eurecam-benjamin

Test program

comment:1 Changed 5 years ago by vadz

  • Milestone set to 3.2

Thanks for finding this bug and for providing the test case!

I'm not too happy about the fix though... Admittedly, current code in source:wxWidgets/trunk/src/msw/ole/dataobj.cpp is already a mess (e.g. we have this hack in IDataObject::SetData() for TYMED_HGLOBAL but not TYMED_ISTREAM which is almost certainly a bug), but copying it around is not the best way forward even so.

Perhaps we could move this code inside wxDataFormat::operator==() instead? I.e. if one of the arguments is wxDF_HTML, get the name of the other one and return true if it's "HTML Format"?

Also, I don't quite understand how do we end with the format different from wxDF_HTML in the common code in the first place, AFAICS the purpose of all these HtmlFormatFixup() calls in MSW-specific part is exactly to prevent this from happening, so perhaps a call to this function is just missing somewhere?

Changed 5 years ago by eurecam-benjamin

A new and cleaner version

comment:2 Changed 5 years ago by eurecam-benjamin

I also think the operators are a much better place for the patch. So I moved the implementation of the operators from msw/ole/dataform.h to msw/ole/dataobj.cpp and inserted the fix here using the existing HtmlFormatFixup function.

They are 4 comparisons, two for wxDataFormat and two for wxDataFormatId. I didn't change the least ones and I don't know if they should be also updated.

Of course the patch still fixes the wxDataObjectComposite problem. It was tested on 3.0.0-rc1 and trunk versions.

I also updated HtmlFormatFixup to avoid unnecessary calls to GetClipboardFormatName. It is just a small optimization which doesn't change the behavior of the function.

comment:3 Changed 5 years ago by vadz

  • Status changed from new to confirmed

Thanks, the new patch indeed looks much better! My only doubt is about not using HtmlFormatFixup() in operator==(wxDataFormatId). Don't we need to apply it to *this here too to ensure that it compares correctly with wxDF_HTML passed in?

comment:4 Changed 5 years ago by VZ

  • Resolution set to fixed
  • Status changed from confirmed to closed

(In [74997]) Fix wxDataFormat comparison operators for wxDF_HTML under wxMSW.

This format is special as it doesn't have a fixed value and is registered
dynamically instead. So we need to call HtmlFormatFixup(), which checks if the
given custom format is actually wxDF_HTML, before comparing formats to ensure
that the real value assigned to this format compares correctly to the fixed
wxDF_HTML value.

Closes #15280.

Note: See TracTickets for help on using tickets.