Closed Bug 81263 Opened 24 years ago Closed 24 years ago

nsIWebBrowserSetup needs a way to switch on/off images

Categories

(Core Graveyard :: Embedding: APIs, defect, P1)

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.1

People

(Reporter: gagan, Assigned: jud)

References

Details

Attachments

(6 files)

Blocks: 69453
I think it's going to be too expensive for the docshell (or some other nsIContentPolicy object w/ a 1to1 relationship with the docshell) to respond to global nsIContentPolicy callbacks. Also, to do this correctly, we need some window context which would mean walking nsIContentPolicies DOM chain up to the nsIDOMWindow, then iterating over all the docshell windows (sub-frames) doing window compares to ensure that the given nsIDOMElement indeed belongs to the given top-level window. would it make sense to have nsIContentPolicy hand out a nsIDOMWindow or nsIDOMDocument also? I'm still wondering if it would be better to have the content policy deal w/ the window model. the idea of multiple windows fielding callbacks that may, or may not, belong to them seems wrong. it seems that this window level context would be helpful. during nsIContentPolicy registration, you could specify an optional nsIDOMWindow param that would indicate you only wants callbacks for the specified window.
I'm not sure if expense is an issue here, but I like the idea of a nsIDOMWindow as a context parameter to NS_CheckContentPolicy. There are several cases I've heard in which embedders want to control policy on a per-window basis.
After talking this over w/ shaver on irc, he wants to keep nsIContentPolicy more global than my suggestion of maintianing a list of windows in it would make it. I can live w/ that; it wasn't designed for window level callbacks, no need to dirty it up. That being the case, we can go one step further and provide the nsIDOMWindow param in the methods to provide more context, but we can also take the existing nsIDOMElement and dig a nsIDOMWindow out of that on the callback side, so maybe we should just do that and tell people wanting window context to do the digging.
I think all I need is for nsIWebBrowserSetup to grow a matching getProperty. Is that doable?
Jud has ok'd adding getProperty to the nsIWebBrowserSetup interface, so I'm going to do that. This bug's mine too, then.
Assignee: valeski → shaver
Severity: normal → major
OS: Linux → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla0.9.1
The attached sample won't work, because it has to go through non-scriptable interfaces and methods (nsIScriptGlobalObject::getDocShell, for one), but it shows what we'll end up doing. I'll hack away on nsIWebBrowserSetup::getProperty now. (Say my name, docshell!)
Status: NEW → ASSIGNED
Well, that's obviously not going to work; I have to ask the docshell for things, because I can't get from it to the web browser setup thing. No biggie, but I just noticed that.
In fact, I don't even need to add that method to nsIWebBrowserSetup. Do you still want it, Jud?
Latest patch builds for me. I can't test it until we get the callsites fixed, but I'll poke again then. (30 lines of C++ in the meat of it. I think that means that Jud and I came to a good decision on the APIs.)
no need for the getproperty on nsIWebbrowsersetup then. coo'
- nsWebBrowserContentPolicy.cpp * the constructor needs to call NS_INIT_ISUPPORTS() in order to be used as an XPCOM component. I'll be posting the allowImages attribute mods to the docshell and the webbrowser setup mods to add the image enum shortly.
Blech. That's just sloppy of me. Sorry. I need IMPL_ISUPPORTS, too. What a loser. I'll post another patch in a little while.
the attatched diffs: - add the allowImages attribute to nsIDocShell and its impl - add the ALLOW_IMAGES enum to nsIWebBrowserSetup (superceeds the nsIWebBrowserSetup.idl mods in shaver's previous patch). - add the ALLOW_IMAGES enum handling to nsWebBrowser.cpp
New patch plays more nicely with XPCOM, and removes the nsIWebBrowserSetup.idl changes in favour of Jud's. Still haven't had a chance to test it with the call-site changes, etc. Giving this to Jud, because I'm a loser, and his tree has most of the goodness.
Assignee: shaver → valeski
Status: ASSIGNED → NEW
Attached patch updated patch. (deleted) — Splinter Review
Summary of most recent patch: - creates a nsWebBrowserContentPolicy class which registers for content policy notifications. as more content policy callbacks come on-line, more than just image blocking will be possible through this. modifies nsIDocShell and nsIWebBrowserSetup to handle the image flag.
sr=rpotts. Just a couple of comments... It looks like you have an extra 'return NS_OK' in UnregisterContentPolicy(...). Also, it would be nice if DeleteCatagoryEntry(...) allowed a null out parameter for the previous catagory entry - that way you wouldn't have to pass in that XPIDLString that jsut gets thrown away :-( Other than that, everything looks good. -- rick
I removed the extra return NS_OK, and I filed http://bugzilla.mozilla.org/show_bug.cgi?id=82000 on the bogus DeleteCategoryEntry param in nsICategoryManager.
last patch is in, and I've added the new files.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
SETUP_ALLOW_IMAGES is in nsIWebBrowserSetup.idl. Support for handling it is in nsWebBrowser.cpp. Image flags in nsDocShell.cpp. Content policy is enforced in nsWebBrowserContentPolicy.cpp (image tag case stmt).
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: