Closed Bug 393503 Opened 17 years ago Closed 17 years ago

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

Categories

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

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

()

Details

(Keywords: regression)

Attachments

(1 file)

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.
Attached patch Proposed fixSplinter Review
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+
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?
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
Closed: 17 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
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.