1055750 - (srcset-mixed-content) Block mixed content <img srcset> and <picture>

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[John Schoenick [:johns]]

The spec was updated to purposefully not define loads through picture and image srcset as blockable. Specifically [1]

> If the element has a srcset attribute or a parent that is a picture element, the fetching request's context must be set to imageset. Otherwise it must be set to image

Where imageset is not defined to be optionally blockable like image [2]

[1] http://www.whatwg.org/specs/web-apps/current-work/multipage/embedded-content.html#update-the-image-data

[2] https://w3c.github.io/webappsec/specs/mixedcontent/#optionally-blockable-content

Comment 1

[John Schoenick [:johns]]

(In reply to John Schoenick [:johns] from comment #0)
> The spec was updated to purposefully not define loads through picture and
> image srcset as blockable.

as optionally-blockable*

Comment 2

[John Schoenick [:johns]]

Attachment: Part 1 - Add content policy TYPE_IMAGESET

Comment 3

[John Schoenick [:johns]]

Attachment: Part 2 - nsContentUtils add optional content policy parameter to nsContentUtils::{Can,}LoadImage

Comment 4

[John Schoenick [:johns]]

Attachment: Part 3 - Track image vs imageset request context through ImageLoadingContent::LoadImage

Comment 5

[John Schoenick [:johns]]

Attachment: Part 3 - Track image vs imageset request context through ImageLoadingContent::LoadImage

Comment 6

[John Schoenick [:johns]]

Attachment: Mixed content tests for imageset

Comment 7

[John Schoenick [:johns]]

Comment on attachment 8498621 [details] [diff] [review]
Part 1 - Add content policy TYPE_IMAGESET

Adds the imageset content policy to definitions, defined in https://fetch.spec.whatwg.org/#requests

Comment 8

[John Schoenick [:johns]]

Comment on attachment 8498622 [details] [diff] [review]
Part 2 - nsContentUtils add optional content policy parameter to nsContentUtils::{Can,}LoadImage

Images may load as content policy TYPE_IMAGE or TYPE_IMAGESET, add a ContentPolicyType flag to nsContentUtils::LoadImage.

This could also be a bool aIsImageSet, but I think this makes callers using imageset more explicit.

(I unrolled the default parameter of nsIContentPolicy::TYPE_IMAGE to two signatures to avoid #include <nsIContentPolicy> in nsContentUtils.h)

Comment 9

[John Schoenick [:johns]]

Comment on attachment 8501364 [details] [diff] [review]
Mixed content tests for imageset

Tests for <img srcset> and <picture> using imageset instead of image as their context -- the later should block mixed content requests.

I added these to the main mixed content test to avoid duplicating too much test framework, but I can split these into a new file if you think it would be cleaner.

Comment 10

[John Schoenick [:johns]]

I should note that these depend on the patches in bug 1037643, the tests fail with odd ordering issues otherwise

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8498622 [details] [diff] [review]
Part 2 - nsContentUtils add optional content policy parameter to nsContentUtils::{Can,}LoadImage

Why can't we include nsIContentPolicy.h exactly?

Comment 12

[John Schoenick [:johns]]

(In reply to Boris Zbarsky [:bz] from comment #11)
> Comment on attachment 8498622 [details] [diff] [review]
> Part 2 - nsContentUtils add optional content policy parameter to
> nsContentUtils::{Can,}LoadImage
> 
> Why can't we include nsIContentPolicy.h exactly?

It adds to the dependency bomb of including nsContentUtils -- I can just go for it if you don't think it's worth the workaround.

Comment 13

[Boris Zbarsky [:bzbarsky]]

I think it's not worth the workaround.

Comment 14

[John Schoenick [:johns]]

Attachment: Part 2 - nsContentUtils add optional content policy parameter to nsContentUtils::{Can,}LoadImage

Dropped the workaround for #include <nsIContentPolicy.h> and just included it

Comment 15

[John Schoenick [:johns]]

Attachment: Part 3 - Track image vs imageset request context through ImageLoadingContent::LoadImage

Comment 16

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8502020 [details] [diff] [review]
Part 2 - nsContentUtils add optional content policy parameter to nsContentUtils::{Can,}LoadImage

r=me

Comment 17

[John Schoenick [:johns]]

Comment on attachment 8502021 [details] [diff] [review]
Part 3 - Track image vs imageset request context through ImageLoadingContent::LoadImage

This calls LoadImage with the Imageset context where appropriate, and has nsImageLoadingContent store REQUEST_IS_IMAGESET with its request flags so it can re-use the proper content policy in cases like ForceReload()

The spec here is pretty straight forward, luckily:
> If the element has a srcset attribute or a parent that is a picture element, the fetching request's context must be set to imageset. Otherwise it must be set to image.

Comment 18

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8502021 [details] [diff] [review]
Part 3 - Track image vs imageset request context through ImageLoadingContent::LoadImage

>+  nsContentPolicyType cpPolicy = PolicyForContext(aImageContext);

Maybe call that policyType?

>+  enum ImageContext {

So I'm not convinced about calling this ImageContext.  Maybe ImageLoadType?  Or ImageSource?

>+      ForceReload();

This makes aNotify automatically true.  I'm not sure we want that in this case...  Can we have an (optional?) argument for controlling this?

r=me with the above addressed

Comment 19

[Tanvi Vyas[:tanvi]]

(In reply to John Schoenick [:johns] from comment #7)
> Comment on attachment 8498621 [details] [diff] [review]
> Part 1 - Add content policy TYPE_IMAGESET
> 
> Adds the imageset content policy to definitions, defined in
> https://fetch.spec.whatwg.org/#requests

You may also need to add TYPE_IMAGESET to the following:
http://mxr.mozilla.org/mozilla-central/source/embedding/browser/nsWebBrowserContentPolicy.cpp#62 
http://mxr.mozilla.org/mozilla-central/source/image/src/imgLoader.cpp#693 - can NewImageChannel() be called on a TYPE_IMAGESET?
http://mxr.mozilla.org/mozilla-central/source/content/html/document/src/ImageDocument.cpp#101
http://mxr.mozilla.org/mozilla-central/source/embedding/browser/nsContextMenuInfo.cpp#283

Please verify that TYPE_IMAGE is appropriate for these cases.

Do you anticipate any significant web compatibility issue with this change, since we will now be blocking TYPE_IMAGESET by default?

I have to check one more thing tomorrow, so leaving the review flag for now.

Comment 20

[Tanvi Vyas[:tanvi]]

Comment on attachment 8498621 [details] [diff] [review]
Part 1 - Add content policy TYPE_IMAGESET

Code looks good, but marking r- until it's confirmed that we don't need to add TYPE_IMAGESET to the files mentioned in comment 19.

Comment 21

[Tanvi Vyas[:tanvi]]

Comment on attachment 8501364 [details] [diff] [review]
Mixed content tests for imageset

>Bug 1055750 - Mixed content tests for imageset
>
This test cycles through all of the possible combinations of blocking/allowing mixed active/display content for multiple content types.  Some don't have onerror/onload events, and hence we've had to use some settimeouts.  Please push to try to see if adding to this test results in any timeouts.


>diff --git a/content/base/test/file_mixed_content_main.html b/content/base/test/file_mixed_content_main.html
>+  // Test 3a: image with srcset
Test 1a, 1b, etc are for mixed active content.  Test 2a, 2b are for mixed display content.  And each section has a comment above it.  Please add some comments for "Part 3" as well:

/* Part 3: Mixed Active Tests for Image srcset */ 

Please also update the comment near the top of the file where it says "<!-- types the Mixed Content Blocker can block"

>+  // Test 3e: Loaded img from <picture> should revert to mixed when leaving the picture
What does this mean exactly?  Revert from mixed display to mixed active?  I'm not clear on what this test is trying to do.

>+  var imgE = document.createElement("img");
>+  var pictureE = document.createElement("picture");
>+  var sourceE = document.createElement("source");
>+  sourceE.srcset = "http://mochi.test:8888/tests/image/test/mochitest/blue.png";
>+  pictureE.appendChild(sourceE);
>+  pictureE.appendChild(imgE);
>+  imgE.src = "http://mochi.test:8888/tests/image/test/mochitest/blue.png";
>+  imgE.onload = imgE.onerror = function() {
>+    // Whether or not it loads, remove it from the picture and observe
>+    pictureE.removeChild(imgE)
>+    imgHandlers(imgE, "imageLeavePicture");
>+  }
>+

Comment 22

[John Schoenick [:johns]]

Attachment: Part 1 - Add content policy TYPE_IMAGESET

(In reply to Tanvi Vyas [:tanvi] from comment #19)
> (In reply to John Schoenick [:johns] from comment #7)
> > Comment on attachment 8498621 [details] [diff] [review]
> > Part 1 - Add content policy TYPE_IMAGESET
> >
> > Adds the imageset content policy to definitions, defined in
> > https://fetch.spec.whatwg.org/#requests
>
> You may also need to add TYPE_IMAGESET to the following:
> http://mxr.mozilla.org/mozilla-central/source/embedding/browser/
> nsWebBrowserContentPolicy.cpp#62

Missed this - fixed

> http://mxr.mozilla.org/mozilla-central/source/image/src/imgLoader.cpp#693 -
> can NewImageChannel() be called on a TYPE_IMAGESET?

I think it can, but only propagates into the LoadInfo struct. Changed this to use the aPolicy load type if aPolicy is passed, defaulting to TYPE_IMAGE as some callers (e.g. loading icons from disk) don't pass a policy. The other callers all appear to get policy type properly from the LoadImage() path or init aPolicy TYPE_IMAGE (which is proper in the non-<img> element case)

> http://mxr.mozilla.org/mozilla-central/source/content/html/document/src/
> ImageDocument.cpp#101

This is okay -- this comes from directly visiting an image, which would not have a srcset/picture context

> http://mxr.mozilla.org/mozilla-central/source/embedding/browser/
> nsContextMenuInfo.cpp#283

This is used for right click -> view background image, which also can't be an imageset context

> Please verify that TYPE_IMAGE is appropriate for these cases.
>
> Do you anticipate any significant web compatibility issue with this change,
> since we will now be blocking TYPE_IMAGESET by default?

IMAGESET will only be used for <img srcset> and <picture> contexts per spec, which we have not shipped yet. Chrome is shipping <picture> and <img srcset> with mixed content blocked currently, though I believe webkit (Safari 7, iOS8) has not yet updated and stil allows mixed <img srcset>. I can ping a few of the blink people and ask if they ran into any issues switching to blocking mixed with <img srcset>, but my impression is that it's not in wide use yet.

Comment 23

[John Schoenick [:johns]]

Attachment: Part 3 - Track image vs imageset request load type through ImageLoadingContent::LoadImage r=bz

Addressed review comments, carrying over r+

(In reply to Boris Zbarsky [:bz] from comment #18)
> Comment on attachment 8502021 [details] [diff] [review]
> Part 3 - Track image vs imageset request context through
> ImageLoadingContent::LoadImage
>
> >+  nsContentPolicyType cpPolicy = PolicyForContext(aImageContext);
>
> Maybe call that policyType?

Fixed
>
> >+  enum ImageContext {
>
> So I'm not convinced about calling this ImageContext.  Maybe ImageLoadType?
> Or ImageSource?

Yes the name here is iffy, there's also "RequestType" floating around this class -- changed this and related calls to ImageLoadType

> >+      ForceReload();
>
> This makes aNotify automatically true.  I'm not sure we want that in this
> case...  Can we have an (optional?) argument for controlling this?

Changed signature to ForceReload(aNotify = true) and passed aNotify here

> r=me with the above addressed

Comment 24

[John Schoenick [:johns]]

Attachment: Mixed content tests for imageset r=tanvi

Updated, carrying over r+ but let me know if you'd like any further changes

(In reply to Tanvi Vyas [:tanvi] from comment #21)
> Comment on attachment 8501364 [details] [diff] [review]
> Mixed content tests for imageset
>
> >Bug 1055750 - Mixed content tests for imageset
> >
> This test cycles through all of the possible combinations of
> blocking/allowing mixed active/display content for multiple content types.
> Some don't have onerror/onload events, and hence we've had to use some
> settimeouts.  Please push to try to see if adding to this test results in
> any timeouts.

This went through try once already at: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=33010414cfab

I will push the full patchset again to try before landing as well

>
> >diff --git a/content/base/test/file_mixed_content_main.html b/content/base/test/file_mixed_content_main.html
> >+  // Test 3a: image with srcset
> Test 1a, 1b, etc are for mixed active content.  Test 2a, 2b are for mixed
> display content.  And each section has a comment above it.  Please add some
> comments for "Part 3" as well:
>
> /* Part 3: Mixed Active Tests for Image srcset */
>
> Please also update the comment near the top of the file where it says "<!--
> types the Mixed Content Blocker can block"

Added comments

> >+  // Test 3e: Loaded img from <picture> should revert to mixed when leaving the picture
> What does this mean exactly?  Revert from mixed display to mixed active?
> I'm not clear on what this test is trying to do.

Added a better comment here - <img src> should be mixed display, while <picture><source>...<img src></picture> should be mixed active, even if we end up using the basic <img src> property as our source. However, upon removing the img from <picture>, it should trigger a new request from its src, which should be downgraded to mixed display.

Comment 25

[John Schoenick [:johns]]

Comment on attachment 8502021 [details] [diff] [review]
Part 3 - Track image vs imageset request context through ImageLoadingContent::LoadImage

[ obsolete by comment 23 ]

Comment 26

[Tanvi Vyas[:tanvi]]

Comment on attachment 8504343 [details] [diff] [review]
Part 1 - Add content policy TYPE_IMAGESET

(In reply to John Schoenick [:johns] from comment #22)
> Created attachment 8504343 [details] [diff] [review]
> Part 1 - Add content policy TYPE_IMAGESET

> > http://mxr.mozilla.org/mozilla-central/source/image/src/imgLoader.cpp#693 -
> > can NewImageChannel() be called on a TYPE_IMAGESET?
> 
> I think it can, but only propagates into the LoadInfo struct. Changed this
> to use the aPolicy load type if aPolicy is passed, defaulting to TYPE_IMAGE
> as some callers (e.g. loading icons from disk) don't pass a policy. The
> other callers all appear to get policy type properly from the LoadImage()
> path or init aPolicy TYPE_IMAGE (which is proper in the non-<img> element
> case)
> 
Getting this right for the LoadInfo struct is important.  CSP and Mixed Content Blocker will use the loadInfo to determine whether to accept/reject redirected requests.  And one day, all of nsIContentPolicy will rely on LoadInfo.

nsIChannelPolicy is almost deprecated - https://bugzilla.mozilla.org/show_bug.cgi?id=1041180.  So using it to determine the content policy type will not work for long.  It would be better if callers passed the content policy type to NewImageChannel(). 


> > Do you anticipate any significant web compatibility issue with this change,
> > since we will now be blocking TYPE_IMAGESET by default?
> 
> IMAGESET will only be used for <img srcset> and <picture> contexts per spec,
> which we have not shipped yet. Chrome is shipping <picture> and <img srcset>
> with mixed content blocked currently, though I believe webkit (Safari 7,
> iOS8) has not yet updated and stil allows mixed <img srcset>. I can ping a
> few of the blink people and ask if they ran into any issues switching to
> blocking mixed with <img srcset>, but my impression is that it's not in wide
> use yet.

Since it's not in wide use yet and hasn't shipped yet, we should be okay! Thanks!

r- for the content policy type issue above.  Thanks for looking into and updating the other places we use TYPE_IMAGE!

Comment 27

[Tanvi Vyas[:tanvi]]

Comment on attachment 8504351 [details] [diff] [review]
Mixed content tests for imageset r=tanvi

(In reply to John Schoenick [:johns] from comment #24)
> Added a better comment here - <img src> should be mixed display, while
> <picture><source>...<img src></picture> should be mixed active, even if we
> end up using the basic <img src> property as our source. However, upon
> removing the img from <picture>, it should trigger a new request from its
> src, which should be downgraded to mixed display.
>
Still lost here.  The test creates an <picture><source srcset><img src></picture>.  The <img src> is used as a fallback, but either way it is considered mixed active content.  When the <img src> is removed, you are left with picture><source srcset></picture>, which is considered mixed active.  Why should the new request revert to mixed display?


>Bug 1055750 - Mixed content tests for imageset
>
>diff --git a/content/base/test/file_mixed_content_main.html b/content/base/test/file_mixed_content_main.html
>index ce73637..36ea642 100644
>+  // Test 3e: img load from <picture> source reverts to img.src as it
>+  //          is removed -- the new request should revert to mixed
>+  //          display policy
>+  var imgE = document.createElement("img");
>+  var pictureE = document.createElement("picture");
>+  var sourceE = document.createElement("source");
>+  sourceE.srcset = "http://mochi.test:8888/tests/image/test/mochitest/blue.png";
>+  pictureE.appendChild(sourceE);
>+  pictureE.appendChild(imgE);
>+  imgE.src = "http://mochi.test:8888/tests/image/test/mochitest/blue.png";
>+  imgE.onload = imgE.onerror = function() {
>+    // Whether or not it loads, remove it from the picture and observe
>+    pictureE.removeChild(imgE)
>+    imgHandlers(imgE, "imageLeavePicture");
>+  }
>+
> </script>
> </body>
> </html>
>diff --git a/content/base/test/test_mixed_content_blocker.html b/content/base/test/test_mixed_content_blocker.html
>+
>+      // Should return to mixed display mode
>+      case "imageLeavePicture":
>+        ok(blockDisplay == (event.data.msg == "insecure image blocked"), "imageLeavePicture did not follow block_active_content pref");
Assuming we should fallback to mixed display, this should say imageLeavePicture did not follow bock_display_content pref

Comment 28

[Simon Pieters [:zcorpan]]

(In reply to Tanvi Vyas [:tanvi] from comment #27)
> Still lost here.  The test creates an <picture><source srcset><img
> src></picture>.  The <img src> is used as a fallback, but either way it is
> considered mixed active content.  When the <img src> is removed, you are
> left with picture><source srcset></picture>, which is considered mixed
> active.  Why should the new request revert to mixed display?

No, you're left with <picture><source srcset></picture> on one hand and the removed <img src> on the other.

<picture><source srcset></picture> does not load anything at all since there's no <img>.

The removed <img src> no longer has <picture> parent so switches to mixed display.

Comment 29

[John Schoenick [:johns]]

Attachment: Part 1 - Add content policy TYPE_IMAGESET

Okay, this version just keeps using TYPE_IMAGE naively in the imgLoader code, so this patch just adds IMAGESET without any consumers. Moved the rest of updating consumers to part 3.5, where they belong anyway.

Comment 30

[John Schoenick [:johns]]

Attachment: [FOLD ME] Part 3.5 - Track content policy type via imgLoad LoadImage and Validate as well

(This will be folded into part 3, but posting separately for review purposes)

This also tracks content policy type through imgLoader, since bug 1041180 nuked nsIChannelPolicy

Comment 31

[John Schoenick [:johns]]

Attachment: Part 2 - nsContentUtils add optional content policy parameter to nsContentUtils::{Can,}LoadImage r=tanvi

Rebased

Comment 32

[John Schoenick [:johns]]

Attachment: Part 3 - Track image vs imageset request load type through ImageLoadingContent::LoadImage r=bz

Rebased

Comment 33

[John Schoenick [:johns]]

Attachment: Mixed content tests for imageset r=tanvi

(In reply to Tanvi Vyas [:tanvi] from comment #27)
> Still lost here.  The test creates an <picture><source srcset><img
> src></picture>.  The <img src> is used as a fallback, but either way it is
> considered mixed active content.  When the <img src> is removed, you are
> left with picture><source srcset></picture>, which is considered mixed
> active.  Why should the new request revert to mixed display?

<picture> will never load an image by itself, rather, it informs the <img> tag residing within it. The actual load here will come from <img src>, which is now loading its src value as mixed display, since it is no longer a member of <picture>


> >+
> >+      // Should return to mixed display mode
> >+      case "imageLeavePicture":
> >+        ok(blockDisplay == (event.data.msg == "insecure image blocked"), "imageLeavePicture did not follow block_active_content pref");
> Assuming we should fallback to mixed display, this should say
> imageLeavePicture did not follow bock_display_content pref

Oops, fixed

Comment 34

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8506412 [details] [diff] [review]
[FOLD ME] Part 3.5 - Track content policy type via imgLoad LoadImage and Validate as well

It doesn't really make sense to add non-trailing optional args in xpidl.  I'm a bit surprised the idl compiler let you...

Please either make this arg non-optional or put it last.  Or make cacheKey optional.

>+    if (!aContentPolicyType) {

We should probably add a value to nsIContentPolicy, something like NO_SUCH_TYPE, mapping to 0, so no one adds a valid 0 value.

r=me

Comment 35

[Tanvi Vyas[:tanvi]]

Comment on attachment 8506410 [details] [diff] [review]
Part 1 - Add content policy TYPE_IMAGESET

># HG changeset patch
># User John Schoenick <[e-mail]>
>
>Bug 1055750 - Part 1 - Add content policy TYPE_IMAGESET
>
>diff --git a/image/src/imgLoader.cpp b/image/src/imgLoader.cpp
>index a96e007..44c441f 100644
>--- a/image/src/imgLoader.cpp
>+++ b/image/src/imgLoader.cpp
>@@ -675,16 +675,17 @@ static nsresult NewImageChannel(nsIChannel **aResult,
>   else {
>     requestingPrincipal = nsContentUtils::GetSystemPrincipal();
>   }
>   nsCOMPtr<nsINode> requestingNode = do_QueryInterface(aRequestingContext);
>   nsSecurityFlags securityFlags = nsILoadInfo::SEC_NORMAL;
>   if (inherit) {
>     securityFlags |= nsILoadInfo::SEC_FORCE_INHERIT_PRINCIPAL;
>   }
>+
White space change looks to be accidently left in this patch.
>   // Note we are calling NS_NewChannelInternal() here with a node and a principal.
>   // This is for things like background images that are specified by user
>   // stylesheets, where the document is being styled, but the principal is that
>   // of the user stylesheet.
>   rv = NS_NewChannelInternal(aResult,
>                              aURI,
>                              requestingNode,
>                              requestingPrincipal,

Comment 36

[Tanvi Vyas[:tanvi]]

Comment on attachment 8506412 [details] [diff] [review]
[FOLD ME] Part 3.5 - Track content policy type via imgLoad LoadImage and Validate as well

># HG changeset patch
># User John Schoenick <[e-mail]>
>
>Bug 1055750 - [FOLD ME] Part 3.5 - Track content policy type via imgLoad LoadImage and Validate as well
>


>diff --git a/image/src/imgLoader.cpp b/image/src/imgLoader.cpp
>index 44c441f..74d13c3 100644
>--- a/image/src/imgLoader.cpp
>+++ b/image/src/imgLoader.cpp

>@@ -2183,19 +2196,20 @@ nsresult imgLoader::LoadImageWithChannel(nsIChannel *channel, imgINotificationOb
>     if (cache.Get(spec, getter_AddRefs(entry)) && entry) {
>       // We don't want to kick off another network load. So we ask
>       // ValidateEntry to only do validation without creating a new proxy. If
>       // it says that the entry isn't valid any more, we'll only use the entry
>       // we're getting if the channel is loading from the cache anyways.
>       //
>       // XXX -- should this be changed? it's pretty much verbatim from the old
>       // code, but seems nonsensical.
>-      if (ValidateEntry(entry, uri, nullptr, nullptr, nullptr, aObserver, aCX,
>-                        requestFlags, false, nullptr, nullptr,
>-                        imgIRequest::CORS_NONE)) {
>+      if (ValidateEntry(entry, uri, nullptr, nullptr, nullptr,
>+                        aObserver, aCX, requestFlags,
>+                        0, false, nullptr,
>+                        nullptr, imgIRequest::CORS_NONE)) {

You are setting the type to zero here because aCanMakeNewChannel is false and hence we don't ever use the content policy type (ValidateRequestWithNewChannel is never called).  I wonder if there is a way to make that clearer?  Adding a comment would be helpful, and also adding something to nsIContentPolicy to cover this case.

Comment 37

[John Schoenick [:johns]]

Attachment: Part 1 - Add content policy TYPE_IMAGESET. r=tanvi

Fixed errant whitespace, carrying over r+

Comment 38

[John Schoenick [:johns]]

Attachment: Part 2 - nsContentUtils add optional content policy parameter to nsContentUtils::{Can,}LoadImage. r=tanvi

Rebased, carrying over r

Comment 39

[John Schoenick [:johns]]

Attachment: Part 3 - Track image vs imageset request load type through ImageLoadingContent::LoadImage r=bz,tanvi

Addressed part 3.5 review comments and folded together with part 3

Comment 40

[John Schoenick [:johns]]

Attachment: Part 3 - Track image vs imageset request load type through ImageLoadingContent::LoadImage r=bz,tanvi

Fix compiling on OS X, widget/cocoa had a LoadImage call that needed TYPE_IMAGE passed

Comment 41

[John Schoenick [:johns]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6bafc792efaa
https://hg.mozilla.org/integration/mozilla-inbound/rev/4892e867616e
https://hg.mozilla.org/integration/mozilla-inbound/rev/2846ff134282
https://hg.mozilla.org/integration/mozilla-inbound/rev/97d6ba17ed2e

Try:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=8af04bb0dc51

Comment 42

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6bafc792efaa
https://hg.mozilla.org/mozilla-central/rev/4892e867616e
https://hg.mozilla.org/mozilla-central/rev/2846ff134282
https://hg.mozilla.org/mozilla-central/rev/97d6ba17ed2e

Comment 43

[Eric Shepherd [:sheppy]]

Updated: https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIContentPolicy

Added to: https://developer.mozilla.org/en-US/Firefox/Releases/36