393503 - [FIX]Disabling images doesn't work for <object> with no type attribute

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

BUILD: Current trunk

STEPS TO REPRODUCE:

1) Disable image loading in preferences
2) Load the URL of this bug

ACTUAL RESULTS: Image shows up

EXPECTED RESULTS: No image.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed fix

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 278005 [details] [diff] [review]
Proposed fix

+  if (NS_FAILED(rv) || NS_CP_REJECTED(shouldProcess)) {
+    HandleBeingBlockedByContentPolicy(rv, shouldProcess);
+    return NS_BINDING_ABORTED;

I think you want to set rv to NS_OK here to avoid the AutoFallback calling Fallback (LoadObject doesn't have the issue because the AutoFallback gets instantiated after the contentpolicy check)

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 278005 [details] [diff] [review]
Proposed fix

Good catch.  Will do that on checkin.

Approvers: this is fairly low-risk, and allows image blocking to work reliably for <object> at least in terms of not showing the image (and for large enough images also not doing most of the load).

Comment 4

[Boris Zbarsky [:bzbarsky]]

Checked in with the rv change there.  Need some tests, somehow...  Ideally something where we set prefs (a la mochitest) then snapshot (a la reftest).

Comment 5

[Jeff Walden [:Waldo]]

Ideally reftest would have a custom profile environment where we could set prefs to allow use of enablePrivilege and such; I suppose that's bug 393410.  With that capability (pun intended!), this would be easy enough to do in a reftest.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Yeah, that would do it.  In general, if we could run reftest like we do mochitest (against an http server, on a new profile) that would be great.