1419657 - Port bug 1415205 |Add add mutate() to nsIURI| to mailnews

Status: RESOLVED FIXED

(MailNews Core :: Backend, enhancement)

Description

[Jorg K (CEST = GMT+2)]

We're busted with 4 unresolved externals:
nsLDAPURL::Mutate()
nsAddbookUrl::Mutate()
nsMsgMailNewsUrl::Mutate()
nsMailtoUrl::Mutate()

Examples here:
https://hg.mozilla.org/mozilla-central/rev/306f999b79a5

Comment 1

[Jorg K (CEST = GMT+2)]

Looks like the boilerplate is this:

C++ file:

+#include "nsIURIMutator.h" // Required, see below.

+NS_IMPL_ISUPPORTS(nsSimpleURI::Mutator, nsIURISetters, nsIURIMutator)
+
+
+NS_IMETHODIMP
+nsSimpleURI::Mutate(nsIURIMutator** aMutator)
+{
+    RefPtr<nsSimpleURI::Mutator> mutator = new XXX::Mutator(); // XXX URI class
+    nsresult rv = mutator->InitFromURI(this);
+    if (NS_FAILED(rv)) {
+        return rv;
+    }
+    mutator.forget(aMutator);
+    return NS_OK;
+}
+

Include file:
+#include "nsIURIMutator.h"

+
+
+public:
+    class Mutator
+        : public nsIURIMutator
+        , public BaseURIMutator<XXX>
+    {
+        NS_DECL_ISUPPORTS
+        NS_FORWARD_SAFE_NSIURISETTERS(mURI)
+        NS_DEFINE_NSIMUTATOR_COMMON
+
+        explicit Mutator() { }
+    private:
+        virtual ~Mutator() { }
+
+        friend class XXX;
+    };

Comment 2

[Jorg K (CEST = GMT+2)]

Attachment: 1419657-nsIURI-mutate.patch (v1) - WIP

Something like this. If this works, compile needs to finish, I'll repeat three times.

Comment 3

[Jorg K (CEST = GMT+2)]

Attachment: 1419657-nsIURI-mutate.patch (v1a) - WIP

Fixed copy/paste errors.

Comment 4

[Jorg K (CEST = GMT+2)]

Attachment: 1419657-nsIURI-mutate.patch (v1b) - WIP

This is the closest I can get to compiling trying to follow the boilerplate.

Valentin, can you please help me out here.

I still get:
0:09.39 c:\mozilla-source\comm-central\obj-i686-pc-mingw32\dist\include\nsIURIMutator.h(61): error C2039: 'Deserialize': is not a member of 'nsMsgMailNewsUrl'
0:09.39 c:\mozilla-source\comm-central\mailnews\base\util\nsMsgMailNewsUrl.h(41): note: see declaration of 'nsMsgMailNewsUrl'
0:09.39 c:\mozilla-source\comm-central\obj-i686-pc-mingw32\dist\include\nsIURIMutator.h(59): note: while compiling class template member function 'nsresult BaseURIMutator<nsMsgMailNewsUrl>::InitFromIPCParams(const mozilla::ipc::URIParams &)'
0:09.39 c:\mozilla-source\comm-central\mailnews\base\util\nsMsgMailNewsUrl.h(60): note: see reference to function template instantiation 'nsresult BaseURIMutator<nsMsgMailNewsUrl>::InitFromIPCParams(const mozilla::ipc::URIParams &)' being compiled
0:09.39 c:\mozilla-source\comm-central\mailnews\base\util\nsMsgMailNewsUrl.h(57): note: see reference to class template instantiation 'BaseURIMutator<nsMsgMailNewsUrl>' being compiled
0:09.39 c:/mozilla-source/comm-central/mozilla/config/rules.mk:1028: recipe for target 'nsMsgMailNewsUrl.obj' failed

Comment 5

[Jorg K (CEST = GMT+2)]

For more context:
https://dxr.mozilla.org/comm-central/rev/7d2c6071fa82d97cd38b06efc23202c0a4e66822/mailnews/base/util/nsMsgMailNewsUrl.h#40

Comment 6

[Jorg K (CEST = GMT+2)]

Adding NI for Daniel since this is pretty urgent.

Comment 7

[Valentin Gosu [:valentin] (he/him)]

Since nsMsgMailNewsUrl doesn't seem to implement nsIIPCSerializableURI then you must make Deserialize return NS_ERROR_NOT_IMPLEMENTED. See https://hg.mozilla.org/mozilla-central/diff/306f999b79a5/caps/NullPrincipalURI.h for an example of how the header would look.

Comment 8

[Jorg K (CEST = GMT+2)]

Attachment: 1419657-nsIURI-mutate.patch (v2) - WIP

This compiles and fixes the link errors for two of the four classes I need to do.

What about the other methods: Read(), SetSpec(). Should they return NS_ERROR_NOT_IMPLEMENTED as shown?

Comment 9

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8930869 [details] [diff] [review]
1419657-nsIURI-mutate.patch (v2) - WIP

Review of attachment 8930869 [details] [diff] [review]:
-----------------------------------------------------------------

Read() is fine as it is, as long as the URL class doesn't implement nsISerializable (it seems it doesn't).

::: mailnews/addrbook/src/nsAddbookUrl.h
@@ +46,5 @@
> +    }
> +
> +    NS_IMETHOD SetSpec(const nsACString & aSpec) override
> +    {
> +      return NS_ERROR_NOT_IMPLEMENTED;

This should return InitFromSpec(aSpec)

::: mailnews/base/util/nsMsgMailNewsUrl.h
@@ +75,5 @@
> +    }
> +
> +    NS_IMETHOD SetSpec(const nsACString & aSpec) override
> +    {
> +      return NS_ERROR_NOT_IMPLEMENTED;

this should return InitFromSpec(aSpec).

Comment 10

[Jorg K (CEST = GMT+2)]

Attachment: 1419657-nsIURI-mutate.patch (v3)

Thanks Valentin, this should be it then.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/ae04ad2124c2
Port bug 1415205 to mailnews: Add add Mutate() to four mailnews URLs. rs=bustage-fix CLOSED TREE

Comment 12

[Jorg K (CEST = GMT+2)]

Looking at the patch now, I see that the indentation is mostly wrong :-(

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/3eb4ce879dc9
Follow-up: Fix incorrect indentation. rs=white-space-only DONTBUILD

Comment 14

[:aceman]

Comment on attachment 8930966 [details] [diff] [review]
1419657-nsIURI-mutate.patch (v3)

Review of attachment 8930966 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks. Yes, the 3-spaces indents are strange.

Comment 15

[Jorg K (CEST = GMT+2)]

Where do you see three? I think I did multiples of two too much which I've removed in the follow up. Is it still not right?

Comment 16

[:aceman]

Yeah, 4 spaces. I shouldn't read deep at night:) And I think you fixed them in the second patch.