Closed Bug 1273849 Opened 3 years ago Closed 3 years ago

Port Bug 1035125 to im and mail. Remove sandboxbroker.dll from Installer

Categories

(Thunderbird :: Build Config, defect, blocker)

defect
Not set
blocker

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 49.0

People

(Reporter: Paenglab, Assigned: aleth)

References

Details

Attachments

(2 files, 3 obsolete files)

sandboxbroker.dll no longer exists after Bug 1035125.

SM has it's own bug.
Attached patch sandbox.patch (obsolete) — Splinter Review
I couldn't test it and I think we don't need port also the parts in /app.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8753803 - Flags: review?(aleth)
Comment on attachment 8753803 [details] [diff] [review]
sandbox.patch

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

No, I think we should port the app/ parts here, as plugins use the sandbox.
Attachment #8753803 - Flags: review?(aleth) → review-
Attached patch sandbox.patch v2 (obsolete) — Splinter Review
I think in TB the plugins are disabled by default => no need for a sandbox.

But I tried to copy it to TB, see the patch. I'm don't know c++ at all and it fails. I'll attach the log.
Attachment #8753803 - Attachment is obsolete: true
Flags: needinfo?(aleth)
Attached file error.log (obsolete) —
Error log of build with patch.
This also updates app/nsMain.cpp with various changes we've missed. That includes bug 1119776, bug 1238769, bug 858928, bug 1272513 and others.
Attachment #8753962 - Flags: review?(mkmelin+mozilla)
Attachment #8753962 - Flags: review?(clokep)
Assignee: richard.marti → aleth
Attachment #8753929 - Attachment is obsolete: true
Attachment #8753931 - Attachment is obsolete: true
(In reply to Richard Marti (:Paenglab) from comment #3)
> I think in TB the plugins are disabled by default => no need for a sandbox.

Most of them are, but Flash is enabled.
Comment on attachment 8753962 [details] [diff] [review]
Port Bug 1035125 and update app/nsMain.cpp

It builds on Win and it works too. I can't test if the container works as I have no message with need for a plugin.
Just a quick drive by, couple of the comment lines mention firefox.exe, presumably this should be changed to reflect the particular application (IM or TB)?
(In reply to Ian Neal from comment #9)
> Just a quick drive by, couple of the comment lines mention firefox.exe,
> presumably this should be changed to reflect the particular application (IM
> or TB)?

The differences of this file from browser/app/nsBrowserApp.cpp are minimal, so I thought it's more useful to minimize the differences that show up in a diff to the relevant ones.
Comment on attachment 8753962 [details] [diff] [review]
Port Bug 1035125 and update app/nsMain.cpp

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

Looks reasonable to me. Thx aleth! r=mkmelin
Attachment #8753962 - Flags: review?(mkmelin+mozilla) → review+
Severity: normal → blocker
Comment on attachment 8753962 [details] [diff] [review]
Port Bug 1035125 and update app/nsMain.cpp

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

I don't see anything crazy. So thanks!
Attachment #8753962 - Flags: review?(clokep) → review+
https://hg.mozilla.org/comm-central/rev/57840632b18aa6ed71b46ed02ecc73d2c3490f5e
Bug 1273849 - Port Bug 1035125 and update app/nsMain.cpp. r=mkmelin,clokep a=aleth CLOSED TREE
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 49.0
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #8754963 - Flags: review?(mkmelin+mozilla)
Attachment #8754963 - Flags: review?(clokep)
Turns out the strangely named kDesktopFolder sets the appSubdir later on, which affects where resource:///modules URIs point. I think this corresponds to the DIST_SUBDIR config var which is not set for c-c:
https://dxr.mozilla.org/comm-central/source/mail/installer/Makefile.in#6

At any rate, I broke nightlies (you get "file not found" errors on startup) and nobody noticed ;)
Attachment #8754963 - Flags: review?(clokep) → review+
Attachment #8754963 - Flags: review?(mkmelin+mozilla) → review+
https://hg.mozilla.org/comm-central/rev/3e9abc46e750f96f75f75b6df736e76cda17db8e
Bug 1273849 - Followup to ensure appSubdir is set correctly for c-c. r=mkmelin,clokep
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Depends on: 1274666
You need to log in before you can comment on or make changes to this bug.