Open Bug 535860 Opened 15 years ago Updated 2 years ago

Cleanup widget/src/windows/nsDataObj

Categories

(Core :: Widget: Win32, defect)

Product:
Core
Component:
Widget: Win32
Platform:
x86
Windows 7
Type:
defect

Tracking

()

People

(Reporter: jimm, Unassigned)

References

Details

What a mess - mixed code formatting, mixed commenting, tabs, and in general the way we handle data the object contains is poorly done. This could be simplified, cleaned up and made to be more general purpose.
We could use some tests too!
Yeah, it's a horrid mess. I've been working in this code (and writing a test or two) in bug 338478 and bug 513464 and trying to keep doing general cleanup in those bugs. I'd be more than happy to take this if you'd like.
(In reply to comment #2) > Yeah, it's a horrid mess. I've been working in this code (and writing a test > or two) in bug 338478 and bug 513464 and trying to keep doing general cleanup > in those bugs. I'd be more than happy to take this if you'd like. You're welcome to it. I was looking at google's implementation of this, although I'm not a fan of their coding style their implementation is much more compact. You might take a look for ideas. We seem to have a lot of bloat, I wonder if we can remove some of that.
(In reply to comment #4) > (In reply to comment #2) > > Yeah, it's a horrid mess. I've been working in this code (and writing a test > > or two) in bug 338478 and bug 513464 and trying to keep doing general cleanup > > in those bugs. I'd be more than happy to take this if you'd like. > You're welcome to it. I was looking at google's implementation of this, > although I'm not a fan of their coding style their implementation is much more > compact. You might take a look for ideas. We seem to have a lot of bloat, I > wonder if we can remove some of that. Yeah, I looked at it to see how they handle drag and drop of multiple things. From what I remember, their code is more compact because a) They don't have a lot of cruft supporting ASCII datatypes for every format b) Their data object implementation is basically just a container object, and the actual logic of generating the HGLOBAL or similar is done elsewhere (in OSExchangeDataProvider or something like that I believe).
Assignee: nobody → me
roc approved bug 484667, Jim if you can check that in before Kyle works on this. That patch enables registering kFilePromiseMime with CF_HDROP. If there is another way to create temp files on the front end then we will have a starting point since that GetData part will be in place.
(In reply to comment #2) > in those bugs. I'd be more than happy to take this if you'd like. I'm going to try my hand at a mochitest for bug 484667 if that helps here. But you don't have to depend on me since I've never done one.
(In reply to comment #7) > (In reply to comment #2) > > in those bugs. I'd be more than happy to take this if you'd like. > > I'm going to try my hand at a mochitest for bug 484667 if that helps here. But > you don't have to depend on me since I've never done one. I don't think you're going to be able to put together a mochitest for 484667 because there's no way to use a mochitest to simulate mouse events outside of the application (firefox/thunderbird/xulrunner etc). It's almost impossible to test interactions with other applications and/or the system. I would suggest writing a litmus test http://quality.mozilla.org/litmus-test-case-writing-primer so testers will know to test that functionality specifically.
Something else worth noting is that the test that I've written only runs on a --disable-libxul build (it uses nonexported symbols) which is never done on tinderbox, so there may be a test in the tree but it isn't being run.
Depends on: 697336
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.