Closed
Bug 81263
Opened 23 years ago
Closed 23 years ago
nsIWebBrowserSetup needs a way to switch on/off images
Categories
(Core Graveyard :: Embedding: APIs, defect, P1)
Core Graveyard
Embedding: APIs
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla0.9.1
People
(Reporter: gagan, Assigned: jud)
References
Details
Attachments
(6 files)
979 bytes,
patch
|
Details | Diff | Splinter Review | |
9.69 KB,
patch
|
Details | Diff | Splinter Review | |
4.06 KB,
patch
|
Details | Diff | Splinter Review | |
1008 bytes,
patch
|
Details | Diff | Splinter Review | |
8.75 KB,
patch
|
Details | Diff | Splinter Review | |
8.81 KB,
patch
|
Details | Diff | Splinter Review |
From bug 69453
Assignee | ||
Comment 1•23 years ago
|
||
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.
Comment 2•23 years ago
|
||
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.
Assignee | ||
Comment 3•23 years ago
|
||
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.)
Assignee | ||
Comment 12•23 years ago
|
||
no need for the getproperty on nsIWebbrowsersetup then. coo'
Assignee | ||
Comment 13•23 years ago
|
||
- 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.
Assignee | ||
Comment 15•23 years ago
|
||
Assignee | ||
Comment 16•23 years ago
|
||
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
Assignee | ||
Comment 17•23 years ago
|
||
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
Assignee | ||
Comment 20•23 years ago
|
||
Assignee | ||
Comment 21•23 years ago
|
||
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.
Comment 22•23 years ago
|
||
r=ccarlen
Comment 23•23 years ago
|
||
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
Assignee | ||
Comment 24•23 years ago
|
||
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.
Assignee | ||
Comment 25•23 years ago
|
||
last patch is in, and I've added the new files.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 26•23 years ago
|
||
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
Updated•5 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•