Since bug #142378, if MixedContentChecker::canDisplayInsecureContent is called with ContentType::ActiveCanWarn then the content is unconditionally allowed, regardless of the browser's mixed content settings. This means that content of type ContentType::ActiveCanWarn can no longer be blocked. We want to support blocking all mixed content, since this is needed for privacy-focused web browsers, so got to check a setting.
But which setting?
There are several options we can pick from:
* We could give up on the semantics of display/run and use display for all content we do not want to block by default, and run for content that we do want to block by default. The security community lingo for this is "passive" and "active" content which sort of maps to the names "display" and "run", but these names are not good because you're hardly "running" a script or a font. Then by default we would disallow "running" of insecure content (including fonts, frames, etc.) but allow "display". This was what I chose to do, but the result is confusing because the allowRunningOfInsecureContent setting now prevents the display of much content. Not a good option because "run" and "display" don't mean what you think they do.
* We can add a third setting to determine how much content to block (strict mode blocks all displayed content, lenient mode allows display of images and videos and "running" of forms). Then by default we would disallow both "display" and "running" of insecure content, but turn off strict mode. That's not a good option because then setting allowDisplayOfInsecureContent and allowRunningOfInsecureContent settings to false will still allow displaying and running some insecure content.
* We can have the canDisplayInsecureContent function check the allowRunningOfInsecureContent setting unless it is of type ActiveCanWarn. Then we can allow "display" but not "running" of insecure content by default, but MixedContentChecker::canDisplayInsecureContent would check the allowRunningOfInsecureContent setting half the time. This one is really confusing!
* Deprecate both existing settings so they do nothing and start over with a clean slate. Remove canDisplayInsecureContent and canRunInsecureContent from MixedContentChecker, and the corresponding functions from FrameLoaderClient. Then we can have just one InsecureContentAllowed setting with the possible values "all" "some" "none" defaulting to "some", and everything will actually make sense. (We could also map the old settings to the new setting, such that display but not run -> "some".) This is the only option I can think of that's not confusing, but I suspect that is not permissible?
Any other ideas? I see no good options; got to pick one though! I think option two is probably the least confusing of the first three, but I don't think the benefit is really worth adding a new setting.
Related: we should also rename ContentType::ActiveCanWarn to ContentType::OptionallyBlockable, since it is currently only used for passive content. That naming looks like it was just a think-o.
(In reply to comment #0)
> these names are not good because you're hardly "running" a script or a
Um, I meant "you're hardly 'running' a frame" (or a web socket or something else we want to block by default besides a script), not "you're hardly 'running' a script."