Closed Bug 428996 Opened 12 years ago Closed 12 years ago

Expose less of the messenger chrome package to content

Categories

(MailNews Core :: Security, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.9

People

(Reporter: philor, Assigned: philor)

References

Details

Attachments

(1 file, 2 obsolete files)

While bug 428767 got us back to working by exposing all of chrome://messenger/ to message content again after bug 292789, that's more than we need to do: unlike browsers wanting to expose global to remote XUL, we have no real desire to expose anything other than messageBody.css to content, and things like bug 390813 to make us want to limit our surface area.

I'm pretty sure all we have to do is register |% content messagebody ... contentaccessible=yes| with an empty directory in messenger.jar for the files we probably don't want to ever have, and then |% skin messagebody| for messageBody.css, and it will still work for third-party themes, but chrome registration and screwups and depend builds don't seem to go together well, so I'm still testing.
I think having one package exposed to content instead of multiple is the way to go. e.g.

content communicator-content jar:comm.jar!/content/communicator-content/ contentaccessible=yes

Put all the files you want exposed in that directory and access them from chrome URLs like chrome://messenger-content/content/messageBody.css

I used this line from comm.manifest as an example:
content communicator-region jar:comm.jar!/content/communicator-region/
Attached patch Thusly? (obsolete) — Splinter Review
Amazing how many typos I managed to make, in so little a patch.
Attachment #315702 - Flags: superreview?(neil)
Attachment #315702 - Flags: review?(neil)
>  +   skin/classic/global/tree/sort-dsc.gif (icons/sort-dsc.gif)

??
Comment on attachment 315702 [details] [diff] [review]
Thusly?

>   skin/classic/messenger/filterDialog.css                               (messenger/filterDialog.css)
>-  skin/classic/messenger/messageBody.css                                (messenger/messageBody.css)
>+% skin messagebody classic/1.0 %skin/classic/messagebody/
>+  skin/classic/messagebody/messageBody.css                              (messenger/messageBody.css)
>  skin/classic/messenger/start.css                                      (messenger/start.css)
At least in suite, can you a) put all the % rules at the top in order and b) put all the CSS files in order i.e.
 % skin global classic/1.0 %skin/classic/global/
+% skin messagebody classic/1.0 %skin/classic/messagebody/
 % skin messenger classic/1.0 %skin/classic/messenger/
...
   skin/classic/editor/icons/progress-failed.gif                         (editor/icons/progress-failed.gif)
+  skin/classic/messagebody/messageBody.css                              (messenger/messageBody.css)
   skin/classic/messenger/dialogs.css                                    (messenger/dialogs.css)
Attachment #315702 - Flags: superreview?(neil)
Attachment #315702 - Flags: superreview+
Attachment #315702 - Flags: review?(neil)
Attachment #315702 - Flags: review+
(In reply to comment #0)
> and it will still work for third-party themes
By "work", you mean "as soon as they move messageBody.css into messagebody/"?

Perhaps stubbing chrome://messagebody/content/messageBody.css works better?

@import url(chrome://messenger/skin/messageBody.css);
Attached patch Less (obsolete) — Splinter Review
Luckily, while I was blathering on about our two choices, Mook came up with a much better third, keeping all the magic out of sight and not moving anyone's food bowl.

(Yeah, I know, override's out of alphabetical order, but as a single logical chunk together they make sense, separated I'd feel the need to add comments explaining what was going on.)
Attachment #315702 - Attachment is obsolete: true
Attachment #316036 - Flags: superreview?(neil)
Attachment #316036 - Flags: review?(neil)
(In reply to comment #6)
> (Yeah, I know, override's out of alphabetical order, but as a single logical
> chunk together they make sense, separated I'd feel the need to add comments
> explaining what was going on.)
Suite style is to list all overrides related to the package with the package.
Attached patch In orderSplinter Review
Conveniently, that also works with Thunderbird style, which is to bung it in there anywhere that it works.
Attachment #316036 - Attachment is obsolete: true
Attachment #316723 - Flags: superreview?(neil)
Attachment #316723 - Flags: review?(neil)
Attachment #316036 - Flags: superreview?(neil)
Attachment #316036 - Flags: review?(neil)
Comment on attachment 316723 [details] [diff] [review]
In order

Actually I wasn't going to be that picky, but you posted a new patch too quickly for me...
Attachment #316723 - Flags: superreview?(neil)
Attachment #316723 - Flags: superreview+
Attachment #316723 - Flags: review?(neil)
Attachment #316723 - Flags: review+
mailnews/jar.mn 1.127
mail/base/jar.mn 1.109
mailnews/mime/emitters/src/nsMimeHtmlEmitter.cpp 1.93
mailnews/mime/emitters/src/nsMimeXmlEmitter.cpp 1.27
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9pre) Gecko/2008042101 SeaMonkey/2.0a1pre] (nightly) (W2Ksp4)

V.Fixed
Status: RESOLVED → VERIFIED
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.