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

RESOLVED FIXED in mozilla1.9.1b3

Status

()

defect
P3
major
RESOLVED FIXED
11 years ago
8 years ago

People

(Reporter: jtd, Assigned: jtd)

Tracking

({fixed1.9.1})

Trunk
mozilla1.9.1b3
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(8 attachments, 8 obsolete attachments)

1.48 KB, text/html
Details
14.87 KB, patch
sicking
: review+
dbaron
: review+
sicking
: superreview+
Details | Diff | Splinter Review
14.87 KB, patch
Details | Diff | Splinter Review
1.74 KB, text/html
Details
860 bytes, text/css
Details
28.32 KB, patch
dbaron
: review+
dbaron
: superreview+
Details | Diff | Splinter Review
28.46 KB, patch
Details | Diff | Splinter Review
959 bytes, patch
jtd
: review+
Details | Diff | Splinter Review
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).
Flags: blocking1.9.1?
Priority: -- → P1
Flags: blocking1.9.1? → blocking1.9.1+
Priority: P1 → P3
You probably want the content policy check regardless of same-origin.

And we might want to add a content policy type for this, too.
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.
tweak to use a distinct name so testcase failure is clearer
Attachment #346206 - Attachment is obsolete: true
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.
Attachment #346211 - Flags: review?(jonas)
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?
(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 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.
Attachment #346211 - Flags: superreview+
Attachment #346211 - Flags: review?(jonas)
Attachment #346211 - Flags: review+
also, please write a reftest and check in, assuming there are free fonts for which that is legal (ahem if nothing better)
Oh, missed the thread about mochitest, and just realized that reftest won't allow you to serve font files cross origin or set headers :(
You can do window snapshots in mochitest and compare them.  See WindowSnapshot.js and the mochitests that use it for examples.
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?
(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...
The problem is that nothing a priori distinguishes a user sheet from any other sheet that is not same-origin with the document...
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.
Attempt to check for whether the referrer URL is system principal or not.  URL's coming from user stylesheets appear to fail this test.
Attachment #346211 - Attachment is obsolete: true
Removed attempt to detect system principal to handle user stylesheet.
Attachment #346381 - Attachment is obsolete: true
Attachment #346384 - Flags: superreview?(jonas)
Attachment #346384 - Flags: review?(jonas)
Attachment #346384 - Flags: review?(dbaron)
Attachment #346384 - Flags: superreview?(jonas)
Attachment #346384 - Flags: superreview+
Attachment #346384 - Flags: review?(jonas)
Attachment #346384 - Flags: review+
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.
Attachment #346384 - Flags: review?(dbaron) → review+
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.
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?
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...
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?
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.
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.
Attachment #346568 - Attachment is obsolete: true
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).
Agent sheets should act like user sheets here.  Doc and override sheets should not.
Load fonts in agent and user stylesheets using the sheet principal, otherwise use the document principal.
Attachment #346632 - Attachment is obsolete: true
Attachment #346838 - Flags: review?(dbaron)
Attachment #346838 - Flags: review?(dbaron) → review?(bzbarsky)
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.
changes based on dbaron comments
Attachment #346838 - Attachment is obsolete: true
Attachment #346847 - Flags: review?(bzbarsky)
Attachment #346838 - Flags: review?(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.
Attachment #346847 - Flags: review?(bzbarsky) → review+
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.
Depends on: 457821
The change to InsertFontFaceRule is easy enough to merge, if that's what you're talking about.
No, I was thinking about the changes needed to pass around the sheetType.
> >   // 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?
Presumably bz meant using nsINetUtil::URIChainHasFlags to check for URI_INHERITS_SECURITY_CONTEXT.
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)
Attachment #347733 - Flags: superreview?(dbaron)
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)
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.
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.
But anything that inherits the principal is by definition not cross-site (you could load it in an iframe and then touch that DOM).
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?..
> 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).
> 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.
> 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 www-style@w3.org, ok?
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 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.
Attachment #347733 - Flags: superreview?(dbaron) → superreview+
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?
Υ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...
Summary: support use of access control headers to allow cross-site downloable fonts → support use of access control headers to allow cross-site downloadable fonts
As suggested.  Used nsFontFaceRuleContainer.
Attachment #347733 - Attachment is obsolete: true
Attachment #350119 - Flags: superreview?(dbaron)
Attachment #350119 - Flags: review?(dbaron)
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.
Attachment #350119 - Flags: superreview?(dbaron)
Attachment #350119 - Flags: superreview+
Attachment #350119 - Flags: review?(dbaron)
Attachment #350119 - Flags: review+
mq patch for checkin
Attachment #346847 - Attachment is obsolete: true
Follow-on patch checked in, changeset 32c9710b2398:
http://hg.mozilla.org/mozilla-central/rev/32c9710b2398
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b3
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.)
Attachment #358628 - Flags: review?(jdaggett)
Comment on attachment 358628 [details] [diff] [review]
remove default value for removed pref

argh, thanks for catching this!
Attachment #358628 - Flags: review?(jdaggett) → review+
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.
You need to log in before you can comment on or make changes to this bug.