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

RESOLVED FIXED

Status

()

Core
DOM
RESOLVED FIXED
10 years ago
5 years ago

People

(Reporter: bz, Assigned: bz)

Tracking

({regression})

Trunk
x86
Linux
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

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.
Created attachment 278005 [details] [diff] [review]
Proposed fix
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #278005 - Flags: superreview?(cbiesinger)
Attachment #278005 - Flags: review?(cbiesinger)
Summary: Disabling images doesn't work for <object> with no type attribute → [FIX]Disabling images doesn't work for <object> with no type attribute
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)
Attachment #278005 - Flags: superreview?(cbiesinger)
Attachment #278005 - Flags: superreview+
Attachment #278005 - Flags: review?(cbiesinger)
Attachment #278005 - Flags: review+
Blocks: 309524
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).
Attachment #278005 - Flags: approval1.9?

Updated

10 years ago
Attachment #278005 - Flags: approval1.9? → approval1.9+
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).
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
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.
Depends on: 393410
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.
No longer blocks: 309524
You need to log in before you can comment on or make changes to this bug.