Closed Bug 1419657 Opened 3 years ago Closed 3 years ago

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

Categories

(MailNews Core :: Backend, enhancement)

enhancement
Not set
blocker

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 59.0

People

(Reporter: jorgk-bmo, Assigned: jorgk-bmo)

Details

Attachments

(1 file, 4 obsolete files)

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
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;
+    };
Summary: Port |Add add mutate() to nsIURI| to mailnews → Port bug 1415205 |Add add mutate() to nsIURI| to mailnews
Something like this. If this works, compile needs to finish, I'll repeat three times.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Fixed copy/paste errors.
Attachment #8930814 - Attachment is obsolete: true
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
Attachment #8930816 - Attachment is obsolete: true
Flags: needinfo?(valentin.gosu)
Adding NI for Daniel since this is pretty urgent.
Severity: normal → blocker
Flags: needinfo?(daniel)
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.
Flags: needinfo?(valentin.gosu)
Flags: needinfo?(daniel)
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?
Attachment #8930828 - Attachment is obsolete: true
Attachment #8930869 - Flags: feedback?(valentin.gosu)
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).
Attachment #8930869 - Flags: feedback?(valentin.gosu) → feedback+
Thanks Valentin, this should be it then.
Attachment #8930869 - Attachment is obsolete: true
Attachment #8930966 - Flags: review?(acelists)
Pushed by mozilla@jorgk.com:
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
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 59.0
Looking at the patch now, I see that the indentation is mostly wrong :-(
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/3eb4ce879dc9
Follow-up: Fix incorrect indentation. rs=white-space-only DONTBUILD
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.
Attachment #8930966 - Flags: review?(acelists) → review+
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?
Yeah, 4 spaces. I shouldn't read deep at night:) And I think you fixed them in the second patch.
Whiteboard: [PLR]
You need to log in before you can comment on or make changes to this bug.