The OS/2 version of nsDragService doesn't use the methods StartDragSession() or EndDragSession() which are inherited from nsBaseDragService. If some code were to call them (in error), they would break our implementation. This patch overrides these two methods and turns them into no-ops to prevent this possibility.

This patch should be applied to the trunk and all applicable branches.

This is a security measure against malitious extensions following bug 285438? Or does it have implications that I don't see because of which I should refresh my unofficial 1.7.6? As a nit, perhaps you could return NS_ERROR_NOT_IMPLEMENTED instead of NS_OK?

(In reply to comment #2) > This is a security measure against malitious extensions following bug 285438? > Or does it have implications that I don't see because of which I should > refresh my unofficial 1.7.6? Not really. This is just protection against future additions to the codebase doing something stupid. The two methods are declared as public, so anyone anywhere could call them in error and bolix up OS/2 d&d. Had they been declared as protected, so that only subclasses of nsBaseDragService could call them, then this wouldn't be needed. WRT the security bug, the OS/2 version of nsDragService had _no_ problems. > As a nit, perhaps you could return NS_ERROR_NOT_IMPLEMENTED instead of NS_OK? Returning an error might break stupid code and give the mistaken impression that our code was at fault. IMHO, it's better to let whatever proceed in blissful ignorance, generating an assertion in the debug version but otherwise doing no harm.

Comment on attachment 178330 [details] [diff] [review] patch to nsDragService for OS/2 Fine, you've convinced me. I hope I am not crossing the line by marking r=me.

Comment on attachment 178330 [details] [diff] [review] patch to nsDragService for OS/2 sr=mkaply

I put this on trunk. It's getting harder to get non security stuff on the branch. If you can come up with a really good reason...

(In reply to comment #6) > I put this on trunk. It's getting harder to get non security stuff on the > branch. If you can come up with a really good reason... This is simply insurance against poorly written new features. If there are no new features added to a branch, no insurance is needed.