457825 - support use of access control headers to allow cross-site downloadable fonts

Status: RESOLVED FIXED

(Core :: Graphics, defect, P3)

Description

[John Daggett (:jtd)]

The downloadable fonts code enforces a same-site origin restriction.  To allow folks control over cross-site access to font files, use the access control headers of the site supplying fonts to permit cross-site access.  Without these headers, the existing same-site origin restriction will be enforced.

spec:
http://www.w3.org/TR/access-control/

(see bug 389508 for related issues).

Note: additional work left over from 441473 - need to assure that full set of content checks are implemented when same-site origin enforcement is manually disabled (see bug 441473, comment 140 and 152).

Comment 1

[Boris Zbarsky [:bzbarsky]]

You probably want the content policy check regardless of same-origin.

And we might want to add a content policy type for this, too.

Comment 2

[John Daggett (:jtd)]

Attachment: testcase, cross-site font linkage with and without access control headers

Testcase shows three lines of text.  The first two link to fonts cross-site.  Only the first has access control headers set so it's the only one that will succeed.  The last one links to a file url so it should always fail except when stored locally.

Comment 3

[John Daggett (:jtd)]

Attachment: testcase, cross-site font linkage with and without access control headers

tweak to use a distinct name so testcase failure is clearer

Comment 4

[John Daggett (:jtd)]

Attachment: patch, v.0.2, enable access control header detection

Wrap nsIStreamLoader object in nsCrossSiteListenerProxy to enable cross-site font downloads when access control headers allow it.

  * policy, principal checks done with document principal
  * referrer passed in from stylesheet data
  * unnecessary includes trimmed from nsCrossSiteListenerProxy.h
  * TYPE_FONT added to nsIContentPolicy types (allows extensions to restrict font downloads)
  * same-site origin pref eliminated

Testcase shows usage.

Comment 5

[Jeff Walden [:Waldo]]

Mochitest can't help you with testing the file: case (yet), but you should be able to do the cross-origin testing without too much difficulty, I think -- will getComputedStyle or something let you check actual font names in use?

Comment 6

[John Daggett (:jtd)]

(In reply to comment #5)
> Mochitest can't help you with testing the file: case (yet), but you should be
> able to do the cross-origin testing without too much difficulty, I think --
> will getComputedStyle or something let you check actual font names in use?

No, information about font matching isn't available via getComputedStyle so there's no way to determine which font ends up getting used.

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 346211 [details] [diff] [review]
patch, v.0.2, enable access control header detection

>@@ -316,19 +316,22 @@ gfxUserFontSet::LoadNext(gfxProxyFontEnt
>                      NS_ConvertUTF16toUTF8(aProxyEntry->mFamily->Name()).get()));            
>             }
>         } 
> 
>         // src url ==> start the load process
>         else {
>             if (gfxPlatform::GetPlatform()->IsFontFormatSupported(currSrc.mURI, 
>                     currSrc.mFormatFlags)) {
>-                PRBool loadOK = mLoaderContext->mLoaderProc(aProxyEntry, 
>-                                                            currSrc.mURI, 
>+                nsresult rv = mLoaderContext->mLoaderProc(aProxyEntry, 
>+                                                            currSrc.mURI,
>+                                                            currSrc.mReferrer,
>                                                             mLoaderContext);

Please fix indentation.

>diff --git a/layout/style/nsCSSRuleProcessor.cpp b/layout/style/nsCSSRuleProcessor.cpp
>--- a/layout/style/nsCSSRuleProcessor.cpp
>+++ b/layout/style/nsCSSRuleProcessor.cpp
>@@ -2398,16 +2398,17 @@ InsertFontFaceRule(nsICSSRule* aRule, gf
>         val.GetStringValue(face->mLocalName);
>         face->mIsLocal = PR_TRUE;
>         face->mURI = nsnull;
>         face->mFormatFlags = 0;
>         break;
>       case eCSSUnit_URL:
>         face->mIsLocal = PR_FALSE;
>         face->mURI = val.GetURLValue();
>+        face->mReferrer = val.GetURLStructValue()->mReferrer;

Would be good if someone who knows the code better could verify that this is correct.


>+nsresult
>+nsFontFaceLoader::CheckLoadAllowed(nsIPrincipal* aSourcePrincipal,
>+                                   nsIURI* aTargetURI,
>+                                   nsISupports* aContext)
...
>+  // check with the security manager
>+  rv = nsContentUtils::GetSecurityManager()->CheckLoadURIWithPrincipal(
>+         aSourcePrincipal, aTargetURI, nsIScriptSecurityManager::STANDARD
>+         );

You don't need to do this, the nsCrossSiteListenerProxy does it for you. Given that this just leaves the contentpolicy check i'd just inline the function, up to you.

>+  // check content policy
>+  PRInt16 shouldLoad = nsIContentPolicy::ACCEPT;
>+  rv = NS_CheckContentLoadPolicy(nsIContentPolicy::TYPE_FONT,
>+                                 aTargetURI,
>+                                 aSourcePrincipal,
>+                                 aContext,
>+                                 EmptyCString(), // mime type
>+                                 nsnull,
>+                                 &shouldLoad,
>+                                 nsContentUtils::GetContentPolicy(),
>+                                 nsContentUtils::GetSecurityManager());

Please pass the document as context, we often pass nodes and it looks like people has found that useful. (currently you are just passing null).


>+  static nsresult CheckLoadAllowed(nsIPrincipal* aSourcePrincipal,
>+                                   nsIURI* aTargetURI,
>+                                   nsISupports* aContext = nsnull);

Nit: generally speaking i'm not a big fan of default arguments. Makes it less explicit what's going on (would have made it more clear that you were passing null as context for example). Feel free to go either way (and is completely moot if you inline function).

Looks good otherwise. r/sr=me, but would be great if you could have someone check out the nsCSSRuleProcessor.cpp stuff.

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

also, please write a reftest and check in, assuming there are free fonts for which that is legal (ahem if nothing better)

Comment 9

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Oh, missed the thread about mochitest, and just realized that reftest won't allow you to serve font files cross origin or set headers :(

Comment 10

[Boris Zbarsky [:bzbarsky]]

You can do window snapshots in mochitest and compare them.  See WindowSnapshot.js and the mochitests that use it for examples.

Comment 11

[Boris Zbarsky [:bzbarsky]]

So I'm a little confused.  We're using the _document_ principal but the _stylesheet_ referrer URI?

Note that this will break font-face in UA/user sheets, since the document will be whatever the document is but presumably the font-face rule is pointing to chrome:// or file://.

I would have thought we'd use the stylesheet principal here, or does this cause problems with people just including a stylesheet cross-site that uses a given font on the other site?

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #11)
> I would have thought we'd use the stylesheet principal here, or does this cause
> problems with people just including a stylesheet cross-site that uses a given
> font on the other site?

Well, that is a case that we're trying to block.

I think we do want to use the style sheet as the referer.  And I think @font-face in user sheets ought to work...

Comment 13

[Boris Zbarsky [:bzbarsky]]

The problem is that nothing a priori distinguishes a user sheet from any other sheet that is not same-origin with the document...

Comment 14

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

yeah, I think for the special case of stylesheet uri being the system principal we want to use it, but otherwise the document principal seems like the right thing.

Comment 15

[John Daggett (:jtd)]

Attachment: patch, v.0.3b, attempt to check for user stylesheets

Attempt to check for whether the referrer URL is system principal or not.  URL's coming from user stylesheets appear to fail this test.

Comment 16

[John Daggett (:jtd)]

Attachment: patch, v.0.3c, don't attempt to check for system principal

Removed attempt to detect system principal to handle user stylesheet.

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 346384 [details] [diff] [review]
patch, v.0.3c, don't attempt to check for system principal

Jonas asked me about the one bit he didn't understand (nsRuleProcessor), which seems right to me.  But you should follow up with bz, who knows this stuff a lot better than I do.

Comment 18

[John Daggett (:jtd)]

Attachment: patch, v.0.3d, fix editing mistake

Comment 19

[John Daggett (:jtd)]

patch v.0.3d checked in
http://hg.mozilla.org/mozilla-central/rev/4671fea63b31

Comment 20

[John Daggett (:jtd)]

Attachment: follow-on patch, v.0.1, special case user stylesheet handling

Implements the following logic in the font loading code:

  if (sheet principal is system principal)
    use sheet principal
  else
    use doc principal

But it looks like @font-face rules in user stylesheets are not loaded with the sheet principal set to the system principal.  Either my logic here is faulty or the loading process has a bug.

Comment 21

[John Daggett (:jtd)]

Also, not sure if the assertion added in the follow-on patch is valid.  Is there some situation under which the sheet principal will be null?

Comment 22

[Boris Zbarsky [:bzbarsky]]

User style sheets don't have a system principal.  That's correct.  As I recall neither do our UA sheets, though we have a bug on changing that.  There were security concerns about making that change for user style sheets...

Comment 23

[John Daggett (:jtd)]

OK, so if the sheet principal is not system principal in the case of user stylesheets, what's the right way to pass down a flag to decide whether to use the sheet principal or the doc principal, to enable user stylesheets to include @font-face rules?  dbaron, ideas/thoughts?

Comment 24

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

If you want you could add an nsStyleSet::sheetType member variable to nsCSSRuleProcessor, passed to its constructor (since the sheet type is sitting right there where its constructor is called), and then propagate that information through to where you need it.

Comment 25

[John Daggett (:jtd)]

Attachment: follow-on patch, v.0.2, use sheetType to flag user stylesheets

As suggested by the comment above, pass the sheetType into the constructor for nsCSSRuleProcessor, then down into InsertFontFaceRule.  If the sheet type is a user stylesheet, use the origin principal, otherwise use the document principal.

Do any other these other types need to act the same way?  I'm assuming "no" for eDocSheet, not sure about the other two types.

    eAgentSheet, // CSS
    eDocSheet, // CSS
    eOverrideSheet, // CSS

Note: needed to dupe the constructor because XBL code creates a nsCSSRuleProcessor.  Not exactly sure what the sheet type should be for that, using eDocSheet for now.

Comment 26

[John Daggett (:jtd)]

Attachment: testcase, fonts with and without access control headers set, plus user stylesheet styles

Comment 27

[John Daggett (:jtd)]

Attachment: user stylesheet for use with testcase

To use the testcase:

1. put the user stylesheet into Profiles/xxx/chrome
2. copy a font into that same folder
3. edit the stylesheet to use that font instead of Verlag-Black.otf
4. load the testcase page

Result: the text should render as described, except the last line should render with the font in step (2).

Comment 28

[Boris Zbarsky [:bzbarsky]]

Agent sheets should act like user sheets here.  Doc and override sheets should not.

Comment 29

[John Daggett (:jtd)]

Attachment: follow-on patch, v.0.3, use sheet principal for agent and user sheets

Load fonts in agent and user stylesheets using the sheet principal, otherwise use the document principal.

Comment 30

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 346838 [details] [diff] [review]
follow-on patch, v.0.3, use sheet principal for agent and user sheets

> nsCSSRuleProcessor::nsCSSRuleProcessor(const nsCOMArray<nsICSSStyleSheet>& aSheets)
>   : mSheets(aSheets)
>   , mRuleCascades(nsnull)
>   , mLastPresContext(nsnull)
>+{
>+  mSheetType = nsStyleSet::eDocSheet;
>+  for (PRInt32 i = mSheets.Count() - 1; i >= 0; --i)
>+    mSheets[i]->AddRuleProcessor(this);
>+}

You should get rid of the old form of the constructor and make all callers pass nsStyleSet::eDocSheet explicitly.  (There are only two, and they're in the XBL code.)

Other than that, passing the review request off to bzbarsky.

Comment 31

[John Daggett (:jtd)]

Attachment: follow-on patch, v.0.3b, change XBL code to explicitly pass sheetType

changes based on dbaron comments

Comment 32

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 346847 [details] [diff] [review]
follow-on patch, v.0.3b, change XBL code to explicitly pass sheetType

>+++ b/gfx/thebes/public/gfxUserFontSet.h
> struct gfxFontFaceSrc {
>     PRPackedBool           mIsLocal;       // url or local
>     nsString               mLocalName;     // full font name if local
>     nsCOMPtr<nsIURI>       mURI;           // uri if url 
>     nsCOMPtr<nsIURI>       mReferrer;      // referrer url if url
>+    nsCOMPtr<nsISupports>  mOriginPrincipal; // principal if url 
>+    
>+    // if url, whether to use the origin principal or not
>+    PRPackedBool           mUseOriginPrincipal;

You probably want to put the two packed bools together so they'll fit in one word of memory.  And if you put them next to the PRUint32, then all three will fit in one word on 64-bit.

>+++ b/layout/style/nsFontFaceLoader.cpp
>+  // if the origin principle is the system principal, use that
>+  // otherwise use the document principal

"origin principal", and no system stuff going on here.  Just using the origin principal if we're supposed to, and the document principal otherwise.

>+  if (aFontFaceSrc->mOriginPrincipal && aFontFaceSrc->mUseOriginPrincipal) {

mOriginPrincipal can't be null here.  I'd be fine asserting that and not checking.

>+    nsCOMPtr<nsIPrincipal> originPrincipal = 
>+      do_QueryInterface(aFontFaceSrc->mOriginPrincipal);    
>+    principal = originPrincipal;

Just QI directly into |principal|; no need for the temporary.

>   if (NS_FAILED(rv)) {
>     nsCAutoString fontURI, referrerURI;
>-    aFontURI->GetSpec(fontURI);
>-    if (aReferrerURI)
>-      aReferrerURI->GetSpec(referrerURI);
>+    aFontFaceSrc->mURI->GetSpec(fontURI);
>+    if (aFontFaceSrc->mReferrer)
>+      aFontFaceSrc->mReferrer->GetSpec(referrerURI);
>     LOG(("fontdownloader download blocked - font uri: (%s) "
>          "referrer uri: (%s) err: %8.8x\n", 
>         fontURI.get(), referrerURI.get(), rv));

The GetSpec stuff should be #ifdef PR_LOGGING and probably conditioned on LOG_ENABLED(), right?

>   // unless data url, open with cross-site listener
>   PRBool isData = PR_FALSE;
>-  if (NS_SUCCEEDED(aFontURI->SchemeIs("data", &isData)) && isData) {
>+  if (NS_SUCCEEDED(aFontFaceSrc->mURI->SchemeIs("data", &isData)) && isData) {

Not relevant to this review, but this looks like it could be a protocol handler flag check instead (for the INHERITS_PRINCIPAL) flag.  I assume you'd want to treat javascript: the same way you do data:, at least, and for the same reasons...  And if we (or an extension) ever add other protocols that behave similarly, things will Just Work.  Followup bug on this?

r=bzbarky with the above nits fixed.

Comment 33

[John Daggett (:jtd)]

Changes to the follow-on patch affect the same area of code as bug 457821.  Since the patch for that bug is somewhat more complex, I think it would be better to have that one land first, then update the follow-on patch to match those changes.

Comment 34

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The change to InsertFontFaceRule is easy enough to merge, if that's what you're talking about.

Comment 35

[John Daggett (:jtd)]

No, I was thinking about the changes needed to pass around the sheetType.

Comment 36

[John Daggett (:jtd)]

> >   // unless data url, open with cross-site listener
> >   PRBool isData = PR_FALSE;
> >-  if (NS_SUCCEEDED(aFontURI->SchemeIs("data", &isData)) && isData) {
> >+  if (NS_SUCCEEDED(aFontFaceSrc->mURI->SchemeIs("data", &isData)) && isData) {
> 
> Not relevant to this review, but this looks like it could be a protocol handler
> flag check instead (for the INHERITS_PRINCIPAL) flag.

Not quite sure what that means, what exactly is the check you're suggesting?

Comment 37

[:Gavin Sharp [email: [e-mail]]]

Presumably bz meant using nsINetUtil::URIChainHasFlags to check for URI_INHERITS_SECURITY_CONTEXT.

Comment 38

[Boris Zbarsky [:bzbarsky]]

PRBool inherits;
rv = NS_URIChainHasFlags(aFontFaceSrc->mURI,
                         nsIProtocolHandler::URI_INHERITS_SECURITY_CONTEXT,
                         &inherits);
if (NS_SUCCEDED(rv) && inherits) {
  // This means aFontFaceSrc->mURI is javascript: or data: or some other URI
  // scheme that has the same behavior of inheriting its principal from its
  // source.
}

(not that I think the comment necessarily needs to say that, by the way; just explaining what the check is checking)

Comment 39

[John Daggett (:jtd)]

Attachment: follow-on patch, v.0.4, changes based on review comments

Comment 40

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Erm, no, i think we specifically want to check for data: here.

What is the use case for supporting anything else? (even data: can IMHO be nixed, but is useful for testing)

Comment 41

[Boris Zbarsky [:bzbarsky]]

Well, the only special thing about data: as far as this code is concerned is that it's really always same-origin with whoever is loading it.  Is there something else?

Basically, any time we have an explicit check for a specific scheme string I see it as a bug.  What's important is how the scheme behaves, and if extensions add new schemes that have the same behavior we should treat them the same.

Comment 42

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

but "how it behaves" is a function of much more than wether it inherits principals or not. It's much more important that it can't reach private cross site copyrighted data for example.

Comment 43

[Boris Zbarsky [:bzbarsky]]

But anything that inherits the principal is by definition not cross-site (you could load it in an iframe and then touch that DOM).

Comment 44

[Sergey Sokoloff]

Question 1.

What happens if not only the font, but also the hypertext page itself is loaded not through HTTP, but by an extension that defined a new scheme?


Question 2.

And what happens if either the hypertext page, or the referenced font, or both are loaded though a new scheme defined not by an extension, but by a mere Web site (see https://developer.mozilla.org/en/Web-based_protocol_handlers for details)? Note that a Web site (unlike an extension) does not even know whether the scheme registration is successful (unless someone fixes bug 440620 which is not likely to be fixed in Firefox 3.1), and thus the Web site have no means to determine which scheme to use.

For example, compare the two following use cases:

1) The reader (the browser's user) visits http://fghi.pp.ru/?area://Smolensk.Info/

2) The reader visits http://fghi.pp.ru/handler.php where navigator.registerProtocolHandler("area", "http://fghi.pp.ru/?%s", "FGHI GaNJa NeTWoRK Gate Area Protocol") is run by the browser and acknowledged by the reader. Then the reader visits area://Smolensk.Info/

There's no DOM for the site to find out whether registerProtocolHandler() was successful (bug 440620). And the URL in the address bar is also the same (or almost the same) in both use cases.

Should the site use area:// URLs or http:// URLs (when the site references some fonts, which it currently does not, but may reference in future when @font-face becomes popular) if the site wants to conform with Firefox same-origin policy? How should site know this?

Not that the site does not use access control headers (and you probably should not force such sites to use them, because the file's on the same site, not on a different one).


Question 3.

1) What if the page is on some HTTP server ("http://...") and the font is on some public FTP ("ftp://.../pub/...")? How should the FTP's master act to indicate that he has no objections against random people downloading fonts from the FTP site?

2) What if the page is on some HTTP server ("http://...") and the font is distributed though a file exchange network ("ed2k://|file|..." or "magnet:...", and imagine you have an extension or a registerProtocolHandler-enabled site to deliver a font from there)?

3) Why don't you act Safari-alike and drop those access control checks altogether? I mean, isn't it easy for the site author to borrow the font from another site by a simple script such as the following few lines of PHP:

ini_set('allow_url_fopen', '1');
header("Content-Type: application/whatever");
 // btw, does anyone knows what the correct MIME for fonts is?
readfile('http://example.org/remote/font/file.ttf');

And the PHP file is in the same domain where the page is. Then what's the reason behind the checking of access control headers?..

Comment 45

[Boris Zbarsky [:bzbarsky]]

> What happens if not only the font, but also the hypertext page itself is loaded
> not through HTTP, but by an extension that defined a new scheme?

Then if the extension uses HTTP as its transport it's up to it to implement nsIHTTPChannel, etc appropriately.  If it's not, it'll just be restricted to same-origin loads (assuming it has a concept of origin different from "the entire URI" of course).

Not sure about the registerProtocolHandler thing, but I suspect that each URI is a separate origin there.

> What if the page is on some HTTP server ("http://...") and the font is on
> some public FTP ("ftp://.../pub/...")?

Then you can't use the font in that page as things stand.  If you expect this to be an issue, please raise it with the folks working on the Access-Control specification.  Same thing for your part 2 of question 3.  Part 3 has been answered before in the www-style mailing list, and I suggest you go look into the archives.  The point here is not to prevent "borrowing" the font, but to reduce cross-site data exposure and to make it hard to accidentally do said "borrowing" (so that the site administrator has to actively decide to engage in copyright infringement if that's what he wishes to do).

Comment 46

[Sergey Sokoloff]

> Then if the extension uses HTTP as its transport it's up to it to implement
> nsIHTTPChannel, etc appropriately.  If it's not, it'll just be restricted
> to same-origin loads (assuming it has a concept of origin different from
> "the entire URI" of course).

> Not sure about the registerProtocolHandler thing, but I suspect that
> each URI is a separate origin there.

I had a hard time trying to find any realistic examples of actual working with non-standard URI schemes. Mozillazine's «Extending the Chrome Protocol» at http://kb.mozillazine.org/Dev_:_Extending_the_Chrome_Protocol suggests redirecting to «data:» URLs, and Rosenberg's «Adding a New Protocol to Mozilla» at http://www.nexgenmedia.net/docs/protocol/ suggests redirecting to «javascript:» URLs, but a real example would involve returning a new custom channel (nsIHTTPChannel or something), altering asyncOpen, using nsIScriptableUnicodeConverter for UTF-8 output, as Brian Fernandes inquired at http://groups.google.com/group/mozilla.dev.extensions/browse_thread/thread/cd03eb3bc26d2e15/67c57fe5d50d16e2 once. I am afraid that most of extension writers are likely to encounter the same problem while trying to sustain the "same origin" state, the whole thing seems underdocumented to me, though my first impression on that matter may be wrong.

> The point here is not to prevent "borrowing" the font, but
> to reduce cross-site data exposure and to make it hard to accidentally do
> said "borrowing" (so that the site administrator has to actively decide
> to engage in copyright infringement if that's what he wishes to do).

It seems interesting, and I foresee a couple of edge cases.

If the server A does readfile() from the server B, it may constitute a copyright infrigement (depending on jurisdiction); but what happens if the above script (running at server A) then gives the font away with access control headers saying "free for all", and then some random buddy from another server D takes it for granted and makes a link from his CSS to that server A? If server A masks the real origin of the font, then the end-users of the font (such as server D) may even be unaware that a copyright infrigement happens.

There are also countries where fonts are not copyrightable/patentable or where international intellectual property is not guarded (countries that never signed Berne Convention, Universal Copyright Convention, etc.). What if both the font-hosting service and the font-hotlinking server reside in Turkmenistan or Tuvalu or Kiribati or Iran or Seychelles, so no copyright infrigement actually happens? It seems that Firefox acts as an agent of U.S./E.U. vision of fonts as objects of intellectual property, but such views are not globally universal, and some people won't be ready to fall in with your views.

Comment 47

[Boris Zbarsky [:bzbarsky]]

> but what happens if the above script (running at server A) then gives the font
> away with access control headers saying "free for all",

IANAL, but seems like server A is not only infringing, but doing so in a fashion likely to lead to severe punitive damages after conviction.  This is no different from just downloading the font from B and putting it up on server A, from a legal POV.  Again, IANAL.

> There are also countries where fonts are not copyrightable/patentable
[etc]

That's fine.  In those countries, someone who appropriates the font carries no legal liability.

> so no copyright infrigement actually happens?

We're not in the business of determining whether infringement is happening, and therefore don't base our behavior on such determinations.  In fact, the copyright issue is not the only reason for the same-origin restriction, as I said above.

Note that the same situation could happen in the US with a freely licensed font, for what it's worth.

Please do go read the extensive existing discussion on the matter on [e-mail], ok?

Comment 48

[Sergey Sokoloff]

Ok, fine.

Now again, how does an extension author define what the same origin policy is for the new URI scheme defined by his extension? This bug (or bug 70132, or some other bug) is probably "dev-doc-needed" unless MDC has an answer. I hope you agree that https://developer.mozilla.org/en/NsIProtocolHandler is not enough in its current state.

Comment 49

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 347733 [details] [diff] [review]
follow-on patch, v.0.4, changes based on review comments

>+  // if the origin principle is the system principal, use that
>+  // otherwise use the document principal

Seems like you missed bzbarsky's comment about this in comment 32.

With that fixed, sr=dbaron.

Comment 50

[John Daggett (:jtd)]

Argh, user stylesheet changes were hosed by dbaron changes for bug 457821, InsertFontFaceRule was moved over to nsPresContext.  Not quite sure how to route the stylesheet type down into that routine.

dbaron, suggestions?

Comment 51

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Υou should probably make a struct (say, nsFontFaceRuleHolder, although that's not a very good name) that has:
  nsRefPtr<nsCSSFontFaceRule> rule;
  sheetType sheetType; /* or PRUint8, or whatever */
and then change all the nsTArray< nsRefPtr<nsCSSFontFaceRule> > to nsTArray<nsFontFaceRuleHolder>.

And perhaps I should have made those array declarations use a typedef -- I was thinking about it, and then decided not to because it would add too much indirection.  Perhaps that was the wrong choice...

Comment 52

[John Daggett (:jtd)]

Attachment: follow-on patch, v.0.5, adjust to changes for bug 457821

As suggested.  Used nsFontFaceRuleContainer.

Comment 53

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 350119 [details] [diff] [review]
follow-on patch, v.0.5, adjust to changes for bug 457821

r+sr=dbaron

You need to fix the forward declaration "class nsFontFaceRuleContainer" to say struct rather than class.  The windows compiler gives an error for struct vs. class mismatch.

Comment 54

[John Daggett (:jtd)]

Attachment: follow-on patch for checkin

mq patch for checkin

Comment 55

[John Daggett (:jtd)]

Follow-on patch checked in, changeset 32c9710b2398:
http://hg.mozilla.org/mozilla-central/rev/32c9710b2398

Comment 56

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: remove default value for removed pref

Thi initial patch here removed support for this pref, but didn't remove it from all.js, so it's still listed in about:config even though it doesn't do anything.  (At least it sure looks like it's been removed.)

Comment 57

[John Daggett (:jtd)]

Comment on attachment 358628 [details] [diff] [review]
remove default value for removed pref

argh, thanks for catching this!

Comment 58

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Landed patch from comment 56:
http://hg.mozilla.org/mozilla-central/rev/7a21bea93924

Comment 59

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Landed patch from comment 56 on 1.9.1:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b97337f6c9c2

Comment 60

[[e-mail]]

Comment on attachment 346411 [details] [diff] [review]
patch, v.0.3d, fix editing mistake

>diff --git a/content/base/public/nsContentPolicyUtils.h b/content/base/public/nsContentPolicyUtils.h
>--- a/content/base/public/nsContentPolicyUtils.h
>+++ b/content/base/public/nsContentPolicyUtils.h
>@@ -135,16 +135,17 @@ NS_CP_ContentTypeName(PRUint32 contentTy
>     CASE_RETURN( TYPE_DOCUMENT          );
>     CASE_RETURN( TYPE_SUBDOCUMENT       );
>     CASE_RETURN( TYPE_REFRESH           );
>     CASE_RETURN( TYPE_XBL               );
>     CASE_RETURN( TYPE_PING              );
>     CASE_RETURN( TYPE_XMLHTTPREQUEST    );
>     CASE_RETURN( TYPE_OBJECT_SUBREQUEST );
>     CASE_RETURN( TYPE_DTD               );
>+    CASE_RETURN( TYPE_FONT              );
...
>diff --git a/content/base/public/nsIContentPolicy.idl b/content/base/public/nsIContentPolicy.idl
>--- a/content/base/public/nsIContentPolicy.idl
>+++ b/content/base/public/nsIContentPolicy.idl
>@@ -120,16 +120,21 @@ interface nsIContentPolicy : nsISupports
>    * Indicates a request by a plugin.
>    */
>   const unsigned long TYPE_OBJECT_SUBREQUEST = 12;
> 
>   /**
>    * Indicates a DTD loaded by an XML document.
>    */
>   const unsigned long TYPE_DTD = 13;
>+
>+  /**
>+   * Indicates a font loaded via @font-face rule.
>+   */
>+  const unsigned long TYPE_FONT = 14;
We probably ought to update nsContentBlocker.cpp as well. I'll file a bug.