1379369 - Port Bug 1364016 [Have _openURIInNewTab() provide the correct triggeringPrincipal] to SeaMonkey

Status: RESOLVED FIXED

(SeaMonkey :: Tabbed Browser, defect)

Description

[Frank-Rainer Grahl (:frg)]

Passing urls to SeaMonkey from the desktop or another programm no longer works. A blank browser tab is opened. Now a triggeringPrincipal needs to be provided.

Comment 1

[Tony Mechelynck [:tonymec]]

The Duplicate This Tab extension might need an upgrade. I'm adding its author to the CC and now I'll post a support request with the extension.

Comment 4

[Frank-Rainer Grahl (:frg)]

Attachment: 1379369-wip.patch

Seems to do the jobs but still needs cleanups (e.g. replace param either implemented or dropped from patch) and maybe further changes.

Comment 5

[Frank-Rainer Grahl (:frg)]

Attachment: 1379369-wip2.patch

Done, I hope.

Need to adjust the comment before asking for review.

Comment 6

[Frank-Rainer Grahl (:frg)]

Attachment: 1379369-wip-beta.patch

A more complete fix but we are still missing principal and referrer usage in quite a few functions. I think this one is it and I will do followup patches before it gets out of hand.

Fixes issues covered in Bug 1379842, Bug 1113431 and Bug 1364016 but we have so many different tab routines that fixing all of it is like an onion and beyond the scope of this bug. I left a few TODOs in the code.

Bug 1379842 changed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9de512d55bb4

Comment 7

[Ian Neal]

Would it be easier to port Bug 1113431 first, then do other work on top of that?

Comment 8

[Frank-Rainer Grahl (:frg)]

Attachment: 1379369-1-groundwork.patch

Base patch to get SeaMonkey at least to a starting level for a port to be possible. It contains a few formatting changes for the later patches. I can seperate them if desired.

Comment 9

[Frank-Rainer Grahl (:frg)]

Attachment: 1379369-1-groundworkbeta-253.patch

Base patch for current comm-beta.

Comment 10

[Frank-Rainer Grahl (:frg)]

Attachment: 1379669-2-triggeringprincipal.patch

Port of Bug Port Bug 1364016. Some parts are already in c-c.

Comment 11

[Frank-Rainer Grahl (:frg)]

Attachment: 1379669-2-triggeringprincipal-253.patch

Port Bug 1364016 for current comm-beta

Comment 12

[Frank-Rainer Grahl (:frg)]

Attachment: 1379369-3-browserdraganddrop.patch

Port Bug 1379369

Comment 13

[Frank-Rainer Grahl (:frg)]

Attachment: 1379369-4-referrerpolicy.patch

Port of Bug Bug 1113431. This needs further work as do the principal use.

I usgesst you test all 4 parts on Beta. 2.54 is currently mostly broken.

Comment 14

[Frank-Rainer Grahl (:frg)]

Attachment: 1379369-1-groundwork.patch

Rebased patch.

Comment 15

[Frank-Rainer Grahl (:frg)]

Attachment: 1379369-2-triggeringprincipal.patch

Rebased patch. Filename also changed.

Comment 16

[Frank-Rainer Grahl (:frg)]

Attachment: 1379369-1-groundworkbeta-253.patch

rebased comm-beta patch

Comment 17

[Frank-Rainer Grahl (:frg)]

Attachment: 1379369-2-triggeringprincipal-253.patch

rebased comm-beta patch

Comment 18

[Ian Neal]

Comment on attachment 8901982 [details] [diff] [review]
1379369-1-groundwork.patch

>+++ b/suite/browser/navigator.js
>@@ -394,31 +394,36 @@ nsBrowserAccess.prototype = {
>                             ? aOpener.document.nodePrincipal.originAttributes.userContextId
>                            : Components.interfaces.nsIScriptSecurityManager.DEFAULT_USER_CONTEXT_ID;
>         let openerWindow = (aFlags & nsIBrowserDOMWindow.OPEN_NO_OPENER) ? null : aOpener;
> 
>         var newTab = gBrowser.loadOneTab(uri, {inBackground: bgLoad,
>                                                fromExternal: isExternal,
>                                                relatedToCurrent: isRelated,
>                                                referrerURI: referrer,
>-                                               userContextId: userContextId,
>+                                               userContextId,
This change doesn't look correct, the Firefox equivalent still has as "userContextId: userContextId,".

>+          if (aURI) {
>+            gBrowser.loadURIWithFlags(aURI.spec, {
>+                                                  flags: loadflags,
>+                                                  referrerURI: referrer,
>+                                                  userContextId,
I cannot see anything in tabbrowser.xml that accepts userContextId or am I missing something?

I think some changes to tabbrowser.xml are also needed to include userContextId.

f+ for the moment

Comment 19

[Frank-Rainer Grahl (:frg)]

Attachment: 1379369-1-groundwork-V2.patch

I changed the assignment but it should not be necessary because the name is the same. Just uses a new reference in the params instead of the original object so I thought I could save a few cpu cycles.

Making user contexts actually work in SeaMonkey is a project for itself but unless private browsing is replaced with a context I see no need to do so:

https://wiki.mozilla.org/Security/Contextual_Identity_Project/Containers

I just put them in to ease further porting and make any checks happy. Most apis now take the parameter. SeaMonkey always uses the default user context for everything. You are better off using a different profile if you need the functionality.

Beta patch not updated for now.

Comment 20

[Frank-Rainer Grahl (:frg)]

Attachment: 1379369-2-triggeringprincipal-V2.patch

rebased

Comment 21

[Frank-Rainer Grahl (:frg)]

Attachment: 1379369-4-referrerpolicy-V2.patch

rebased

Comment 22

[Ian Neal]

Comment on attachment 8903925 [details] [diff] [review]
1379369-1-groundwork-V2.patch

LGTM r=me

Comment 23

[Ian Neal]

Comment on attachment 8903926 [details] [diff] [review]
1379369-2-triggeringprincipal-V2.patch

LGTM r=me

Comment 24

[Ian Neal]

Comment on attachment 8900301 [details] [diff] [review]
1379369-3-browserdraganddrop.patch

LGTM r=me

Comment 25

[Ian Neal]

Comment on attachment 8903927 [details] [diff] [review]
1379369-4-referrerpolicy-V2.patch

LGTM r=me

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/008b94f56181
Part 1. Base patch passing a triggeringPricipal and userContextId to more parts in the tabbrowser. r=IanN
https://hg.mozilla.org/comm-central/rev/921b8e3b2731
Part 2. Port Bug 1364016 [Explicitly pass a triggeringPrincipal to openURI] to SeaMonkey. r=IanN
https://hg.mozilla.org/comm-central/rev/7bdcbbdd0e1f
Part 3. Port Bug 1379842 [Check content principal when dragging and dropping from browser] to SeaMonkey. r=IanN
https://hg.mozilla.org/comm-central/rev/a202c95f898c
Part 4. Port part of Bug 1113431 [Propagate referrer policy throughout the UI] to SeaMonkey. r=IanN

Comment 27

[Frank-Rainer Grahl (:frg)]

Attachment: 1379369-1-groundworkbeta-V2-253.patch

Rebased patch for 2.53 comm-beta. r+ from IanN carried forward.

[Approval Request Comment]
Regression caused by (bug #): Bug 1364016
User impact if declined: external url handling broken.
Testing completed (on m-c, etc.): c-b
Risk to taking this patch (and alternatives if risky): none broken already.
String changes made by this patch: --

Comment 28

[Frank-Rainer Grahl (:frg)]

Attachment: 1379369-2-triggeringprincipal-V2-253.patch

Rebased patch for 2.53 comm-beta. r+ from IanN carried forward.

[Approval Request Comment]
Regression caused by (bug #): Bug 1364016
User impact if declined: external url handling broken.
Testing completed (on m-c, etc.): c-b
Risk to taking this patch (and alternatives if risky): none broken already.
String changes made by this patch: --

Comment 29

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8903927 [details] [diff] [review]
1379369-4-referrerpolicy-V2.patch

[Approval Request Comment]
Regression caused by (bug #): Bug 1364016
User impact if declined: external url handling broken.
Testing completed (on m-c, etc.): c-b
Risk to taking this patch (and alternatives if risky): none broken already.
String changes made by this patch: --

Part 3 is 2.54+ and must not be checked into Beta.

Comment 30

[K Chayka]

Seamonkey Build identifier: 20170911061039 is still broken. If a browser window is already open, links from 3rd party apps always open a new window and show the home page regardless of pref settings.

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/7fe78c5eaf64
Part 5. Remove undefined UserContextID in default open uri path. rs=bustage-fix

Comment 32

[Frank-Rainer Grahl (:frg)]

Attachment: 1379369-5-userContextId-fix.patch

r+ bustage fix.

In one case the UserContextId was undefined. Pre 2.54 didn't care but recent changed exposed the bug. See screenshot in Comment 30.

Rather than refactoring the patches for 2.53 I would like them to match the ones in 2.54 and push the bustage fixe there too if approved.

Comment 33

[Frank-Rainer Grahl (:frg)]

K Chayka would you like to test a 2.53 or 2.54 version with all the work patches still in review applied? I tested this for 2.53 a few weeks ago and it did work for me. Might be new breakage. If not I will see that I check it over the weekend.

Comment 34

[K Chayka]

frg: sure - I'd be glad to test a 2.54 build for Win64. Where can I get one?

Comment 35

[Ian Neal]

Comment on attachment 8903927 [details] [diff] [review]
1379369-4-referrerpolicy-V2.patch

a=me

Comment 36

[Ian Neal]

Comment on attachment 8906402 [details] [diff] [review]
1379369-1-groundworkbeta-V2-253.patch

a=me

Comment 37

[Ian Neal]

Comment on attachment 8906403 [details] [diff] [review]
1379369-2-triggeringprincipal-V2-253.patch

a=me

Comment 38

[Ian Neal]

Comment on attachment 8907069 [details] [diff] [review]
1379369-5-userContextId-fix.patch

a=me

Comment 39

[Frank-Rainer Grahl (:frg)]

Part 1
https://hg.mozilla.org/releases/comm-beta/rev/55d03c8b460ac66acdea66c0fa47790145fda3a9
Part 2
https://hg.mozilla.org/releases/comm-beta/rev/b143eb9b2e3ca092d3731916600429334d6b18eb
Part 4
https://hg.mozilla.org/releases/comm-beta/rev/c7645d8a5b8ea3c4369825cefc34fc2ce4a3f340
Part 5
https://hg.mozilla.org/releases/comm-beta/rev/ef728a66d5c00574dd0aa23eac4c6ab25b4ea96a

Comment 41

[Frank-Rainer Grahl (:frg)]

Comment on attachment 8900301 [details] [diff] [review]
1379369-3-browserdraganddrop.patch

Just for reference. If someone builds 2.53 from a 56 release tree this one needs to go in. Bug 1379842 was backported to Ggecko 56 and I also made a mistake in part 1 where I left the triggeringPrincipal in the code and didn't intialize it.

Comment 42

[Frank-Rainer Grahl (:frg)]

Attachment: 1379369-4-nsBrowserAccess.patch

Another update for 2.53 only. 

Links with target="_blank" did ignore the configured link behaviour and open in a new window. Bad backport for 2.53 only. The removed lines are correct for 2.54 and up so this does not need a followup bug. 

An example is clicking on a release notes link here: https://www.visualstudio.com/downloads/

Comment 43

[Frank-Rainer Grahl (:frg)]

Target 2.53.1
https://gitlab.com/seamonkey-project/seamonkey-2.53-comm/-/commit/093b74cf9de7e49c05f25a642fa0cc98ea12d184
Part 3. Port Bug 1379842 [Check content principal when dragging and dropping from browser] to SeaMonkey. r=IanN

https://gitlab.com/seamonkey-project/seamonkey-2.53-comm/-/commit/b6e9d83148030544cbf825d0766f70ddff62bf21
Part 4. SeaMonkey 2.53 only. Remove invalid check for null uri.