nsISupportsArray can be converted to nsIArray in this function. /mozilla/editor/nsIHTMLEditor.idl line 490 -- nsISupportsArray getLinkedObjects(); /mozilla/editor/libeditor/nsHTMLEditor.cpp line 45 -- #include "nsISupportsArray.h" line 2739 -- nsHTMLEditor::GetLinkedObjects(nsISupportsArray** aNodeList) /editor/ui/dialogs/content/EdLinkChecker.js line 54 -- objects = editor.getLinkedObjects();

Jorg, would you have an idea where this "LinkChecker" is used?

This is to be applied on top of bug 1272893.

Successful try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=ec896cdee6739854c7cc753e268e9998f8f68b4d

Comment on attachment 8752655 [details] [diff] [review] patch for c-c No, I don't. If it's not used in "Insert > Link" (have you tried?), then it might be used only by SM. Ask Philip. Your changes look plausible, but if you're telling me that we can't execute this code, I prefer not to f+ them ;-)

(In reply to Jorg K (PTO during summer, NI me) from comment #4) > No, I don't. If it's not used in "Insert > Link" (have you tried?), then it > might be used only by SM. Ask Philip. I tried insert link, but it uses another xul file. > Your changes look plausible, but if you're telling me that we can't execute > this code, I prefer not to f+ them ;-) Yes, I think I didn't execute the code so I'm looking for a way to test it :)

(In reply to :aceman from comment #5) > (In reply to Jorg K (PTO during summer, NI me) from comment #4) > > No, I don't. If it's not used in "Insert > Link" (have you tried?), then it > > might be used only by SM. Ask Philip. > I tried insert link, but it uses another xul file. > > > Your changes look plausible, but if you're telling me that we can't execute > > this code, I prefer not to f+ them ;-) > Yes, I think I didn't execute the code so I'm looking for a way to test it :) I don't know either. Maybe fabien (nvu/kompozer) would know how to test this code.

Comment on attachment 8752656 [details] [diff] [review] patch for m-c Review of attachment 8752656 [details] [diff] [review]: ----------------------------------------------------------------- I don't own the editor any more. Forwarding to Masayuki.

Comment on attachment 8752656 [details] [diff] [review] patch for m-c > NS_IMETHODIMP >-nsHTMLEditor::GetLinkedObjects(nsISupportsArray** aNodeList) >+nsHTMLEditor::GetLinkedObjects(nsIArray** aNodeList) > { > NS_ENSURE_TRUE(aNodeList, NS_ERROR_NULL_POINTER); > >+ nsresult rv; >+ nsCOMPtr<nsIMutableArray> nodes = do_CreateInstance(NS_ARRAY_CONTRACTID, &rv); >+ NS_ENSURE_SUCCESS(rv, rv); > > nsCOMPtr<nsIContentIterator> iter = >- do_CreateInstance("@mozilla.org/content/post-content-iterator;1", &res); >+ do_CreateInstance("@mozilla.org/content/post-content-iterator;1", &rv); nit: Odd indent. Please use two spaces for indent. > NS_ENSURE_TRUE(iter, NS_ERROR_NULL_POINTER); >- if ((NS_SUCCEEDED(res))) >+ if (NS_SUCCEEDED(rv)) > { Could you put the |{| to the end of the line for |if (NS_SUCCEEDED(rv))|? > interface nsIAtom; > interface nsIContent; >-interface nsISupportsArray; >+interface nsIArray; Hmm, you need this forward decl, you must need to include nsIArray.h in the .cpp file, isn't it? With those changes, I'll give r+, but I'd like to check the next patch. So, temporary -'ing. # Sorry for the delay to review. I dropped this review request due to confused with similar bug's review request. Feel free to ping me if I don't review in a couple of days.

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) (Offline: 9/19, 9/22-9/25, 9/28)) from comment #8) > Comment on attachment 8752656 [details] [diff] [review] > patch for m-c > > interface nsIAtom; > > interface nsIContent; > >-interface nsISupportsArray; > >+interface nsIArray; > > Hmm, you need this forward decl, you must need to include nsIArray.h in the > .cpp file, isn't it? > nsIArray.h is included via nsIMutableArray.h .

Thanks, fixed the nits. This one again has the decision about whether to append weak or strong references.

It seems this link checker is used in the page composer/editor (not email composer), which is available in SM only and the standalone external composers (nvu). Ian, could you please check both patches in Seamonkey?

Comment on attachment 8792210 [details] [diff] [review] patch for m-c v1.1 >- nsresult res; >- >- res = NS_NewISupportsArray(aNodeList); >- NS_ENSURE_SUCCESS(res, res); >- NS_ENSURE_TRUE(*aNodeList, NS_ERROR_NULL_POINTER); >+ nsresult rv; Nice! >+ nsCOMPtr<nsIMutableArray> nodes = do_CreateInstance(NS_ARRAY_CONTRACTID, &rv); >+ NS_ENSURE_SUCCESS(rv, rv); Please use NS_WARN_IF() when you rewrite NS_ENSURE_*() in editor. So, this should be: if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } >- res = NS_NewHTMLURIRefObject(getter_AddRefs(refObject), node); >- if (NS_SUCCEEDED(res)) >+ rv = NS_NewHTMLURIRefObject(getter_AddRefs(refObject), node); >+ if (NS_SUCCEEDED(rv)) > { Please put this |{| at the end of previous line because you are touching the previous line.

Thanks.

Comment on attachment 8792211 [details] [diff] [review] patch for c-c v1.1 LGTM r/a=me Logged Bug 1304542 for nsIURIChecker issue.

Aleth, can you do the coordinated checkin into m-c and c-c? Thanks.

If Aleth pushes the M-C part to inbound, I can land the C-C part when I see the merge of the M-C part to M-C ... given that I'm CC'ed on the bug - unlike what happened with bug 1300562 :-(

https://hg.mozilla.org/integration/mozilla-inbound/rev/37ed8d70f7b2c0a944903d2d4b1e4a5e5ac6a92c Bug 1273001 - convert HTMLEditor::getLinkedObjects() to nsIArray. r=masayuki

Thanks. I've pushed the c-c myself now. https://hg.mozilla.org/comm-central/rev/6c4f1970f8fe50cbc3fec4678a55657ef4573afb The c-c changes are in JS so it will not break the build even if we end up without the m-c part for several hours. Leaving NI for aleth so he can paste the final m-c link.