Open Bug 176166 Opened 18 years ago Updated 11 months ago

nsIDocShell::appType is not easily extensible

Categories

(Core :: DOM: Navigation, defect)

defect
Not set

Tracking

()

People

(Reporter: darin.moz, Unassigned)

Details

nsIDocShell::appType is not easily extensible.  some of our security code is app
specific (e.g., javascript disabled by default in mailnews).  code inside
nsScriptSecurityManager checks the docshell's appType attribute.  since other
apps probably care about security too, it seems likely that other apps would
want to be able to likewise define themselves as an nsIDocShell::appType, and
then modify code (perhaps inside caps) to taylor moz security for their app. 
but this is not an option for apps that are not part of the mainstream mozilla
release.
It's hard to see why apptype even exists.

What's wrong with testing what the docshell supports via the various "allow"
attributes, e.g. allowJavascript, allowImages, allowSubframes, allowPlugins etc.?

I only see only potentially useful place where it's called in nsGlobalWindow.cpp

http://lxr.mozilla.org/seamonkey/search?string=etAppType

If the attribute has to live on then it could always be turned into a string to
be more flexible.
Right, you are, Adam... The current users of GetAppType are:

nsImgManager.cpp -- this should use GetAllowImages() (and mailnews needs to
                    handle the pref stuff itself in
                    nsMsgWindow::SetRootDocShell)
nsScriptSecurityManager.cpp -- this should use GetAllowJavascript (again,
                               mailnews needs to handle the prefs)
nsGlobalWindow.cpp -- this may need a new boolean flag -- "expose as opener" or
                      something.  Maybe this could become a bitfield if we end
                      up with too many flags?
nsXMLElement.cpp -- this is checking whether auto-links are allowed... perhaps
                    this should just use the meta refresh boolean?

That's all the callers.  I think we should just convert them to use the booleans
Adam pointed out and nix the apptype thing.
Assignee: adamlock → nobody
QA Contact: adamlock → docshell
Nothing like resurrecting a 16 year old bug...

I'm currently converting nsIDocShell const lists to CEnums in bug 1505601, and just ran into APP_TYPE. AFAICT, in m-c we never set AppType to anything but APP_TYPE_UNKNOWN unless it's in tests, and APP_TYPE_MAIL is pretty much invalid for m-c now anyways. It seems like we could remove this.

However, c-c still uses APP_TYPE heavily, so removing this from nsIDocShell is going to cause them some friction.

:bz, any thoughts on how to handle this?
Flags: needinfo?(bzbarsky)
Assignee: nobody → kyle
As you note, c-c relies on appType pretty heavily.  That's the only reason it ever existed, in fact.

Is there a need to remove it, as opposed to just switching to a CEnum?  If so, seems like we need to have  some  way to indicate the same information...
Flags: needinfo?(bzbarsky)
That said, does c-c ever _read_ the appType?  If it just sets it, we can look at the very small number of places in m-c that it gets read and come up with some other ways for c-c to get that behavior.  That's sort of what comment 2 was about, I think.   It's been a while. ;)
c-c reads it once, from what I can tell. If we came up with another place to put it, we could just convert that too.

Either way, for now I'm just converting it to a cenum, and will add a "do not delete" comment there, while leaving this bug open. I'm not really sure trying to get all of the required work synced between gecko and c-c teams would be worth the effort right now. I was mostly curious about how we should deal with an issue of split usage between the repos like this, and wanted to make sure there's notes in the code so someone else doesn't try to clean it up.
(and it's switching to CEnum in bug 1505601)
Assignee: kyle → nobody
You need to log in before you can comment on or make changes to this bug.