Port Bug 1513241 Pass loadURIOptions dictionary for docshell loads

RESOLVED FIXED in Thunderbird 66.0

Status

enhancement
RESOLVED FIXED
4 months ago
4 months ago

People

(Reporter: jorgk, Assigned: jorgk)

Tracking

unspecified
Thunderbird 66.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Assignee

Description

4 months ago

Instead of passing single parameters, this now passes an object:
https://hg.mozilla.org/mozilla-central/rev/71cb45922e34#l1.12

We have a few call-sites
https://searchfox.org/comm-central/search?q=webnav.*LoadURI&case=false&regexp=true&path=
in C++ and JS code.

Assignee

Comment 1

4 months ago
Posted patch 1519433-loaduri.patch (v0) (obsolete) — Splinter Review

This compiles but the JS part is missing. Coming up.

Assignee

Comment 2

4 months ago

First reviewer wins. I couldn't test chat since it's broken (bug 1519091).

Attachment #9035947 - Attachment is obsolete: true
Attachment #9035954 - Flags: review?(mkmelin+mozilla)
Attachment #9035954 - Flags: review?(geoff)
Attachment #9035954 - Flags: review?(acelists)

Comment 3

4 months ago

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/77f1eb52e155
Port bug 1513241: Adjust parameters of nsIWebNavigation.loadURI(). rs=bustage-fix

Status: NEW → RESOLVED
Last Resolved: 4 months ago
Resolution: --- → FIXED
Assignee

Updated

4 months ago
Target Milestone: --- → Thunderbird 66.0
Comment on attachment 9035954 [details] [diff] [review]
1519433-loaduri.patch (v1)

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

r=mkmelin

::: mailnews/base/src/nsMsgWindow.cpp
@@ +505,5 @@
>  
>    nsCOMPtr<nsIWebNavigation> webNav(do_QueryInterface(docShell));
>    NS_ENSURE_TRUE(webNav, NS_ERROR_FAILURE);
>  
> +  mozilla::dom::LoadURIOptions loadURIOptions;

slightly awkward not to just add the needed namescape on top and use LoadURIOptions
Attachment #9035954 - Flags: review?(mkmelin+mozilla)
Attachment #9035954 - Flags: review?(geoff)
Attachment #9035954 - Flags: review?(acelists)
Attachment #9035954 - Flags: review+
Assignee

Comment 5

4 months ago

Well practice is to copy as much as we can from core, right? And they seem to use namespaces.
Someone more c++ could chim in, but is there any reason we shouldn't use them?

You need to log in before you can comment on or make changes to this bug.