203062 - missing drop hook and move to utility methods

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[Kathleen :Brade]

We are missing a callback to drophook in DragOver.
Also, we should move all of this hook stuff to nsEditorUtils.cpp.

Comment 1

[Kathleen :Brade]

Attachment: missing hook v1

Comment 2

[Simon Fraser [no longer active]]

Comment on attachment 121434 [details] [diff] [review]
missing hook v1

>Index: libeditor/base/nsEditorUtils.cpp
>===================================================================

>+/******************************************************************************
>+ * utility methods for drag/drop/copy/paste hooks
>+ *****************************************************************************/
>+
>+nsresult
>+nsEditorHookUtils::GetHookEnumeratorFromDocument(nsIDOMDocument *aDoc,
>+                                                 nsISimpleEnumerator **aResult)
>+{
>+  nsCOMPtr<nsIDocument> doc = do_QueryInterface(aDoc);
>+  if (!doc) return NS_ERROR_FAILURE;
>+
>+  nsCOMPtr<nsISupports> isupp;
>+  doc->GetContainer(getter_AddRefs(isupp));
>+  nsCOMPtr<nsIDocShell> docShell = do_QueryInterface(isupp);
>+  nsCOMPtr<nsIClipboardDragDropHookList> hookObj = do_GetInterface(docShell);
>+  if (!hookObj) return NS_ERROR_FAILURE;
>+
>+  nsCOMPtr<nsISimpleEnumerator> enumerator;
>+  hookObj->GetHookEnumerator(getter_AddRefs(enumerator));
>+  if (!enumerator) return NS_ERROR_FAILURE;
>+
>+  *aResult = enumerator;
>+  NS_ADDREF(*aResult);
>+  return NS_OK;
>+}

You could just 
  return hookObj->GetHookEnumerator(aResult);
here.

Otherwise it looks good!

Comment 3

[kinmoz]

Comment on attachment 121434 [details] [diff] [review]
missing hook v1

r=[e-mail] with sfraser's issue addressed.

My one concern is that this change will prevent embedders from ever extending
the types of data that can be dropped into the editor, for example if they had
the ability to convert RTF to HTML, since the editor will deny the drop and
never give the hooks a crack at saying yes. I guess we can file another bug to
track that issue?


   PRBool canDrop = CanDrop(aDragEvent);
+  if (canDrop)
+  {
+    nsCOMPtr<nsIDOMDocument> domdoc;
+    mEditor->GetDocument(getter_AddRefs(domdoc));
+    canDrop = nsEditorHookUtils::DoAllowDropHook(domdoc, aDragEvent,
dragSession);
+  }
+

Comment 4

[Kathleen :Brade]

Attachment: updated patch

Comment 5

[kinmoz]

FYI, I filed bug 203105 to track the CanDrop()/DoAllowDropHook() issue I
mentioned above.

Comment 6

[Kai Engert [:KaiE:]]

Comment on attachment 121469 [details] [diff] [review]
updated patch

r=kaie on the differences between this patch and the previous version -
carrying forward the other review

Comment 7

[Kathleen :Brade]

I would like to land this patch for 1.4beta.  It cleans up the editor code
(better factoring) for dealing with the hooks that initially landed as part of
bug 197121.  I'd like this to land for 1.4beta for editor embedding.

Comment 8

[Asa Dotzler [:asa]]

Comment on attachment 121469 [details] [diff] [review]
updated patch

a=asa (on behalf of drivers) for checkin to 1.4b

Comment 9

[Kathleen :Brade]

patch checked in

Comment 10

[(not reading, please use [e-mail] instead)]

fixed, so clearing blocker request.

Comment 11

[sairuh (rarely reading bugmail)]

i did some simple d'n'd btwn editor windows, mail compose and a browser window,
and this looks fine. let me know if there's a particular scenario[s] i should
check out --but otherwise this looks fine with 2003.05.13 builds.