77390 - should display <noscript> content if nsScriptSecurityManager::CanExecuteScripts returns false

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Eric Pollmann]

We currently display <noscript> content only if the 'javascript.enabled' pref is
set to false.  We should instead display <noscript> content if the script
security manager disables script for any reasone (e.g.,
nsDocShell::SetAllowJavascript(PR_FALSE))  This will ensure that <noscript>
descriptive text is displayed for content whenever scripts are not running.

This is a follow-on to bug 77227

Comment 1

[Eric Pollmann]

Attachment: patch, check security manager rather than prefs + cleanup

Comment 2

[Eric Pollmann]

The reason I split this check out into a static function and used it in two
places is because in HTMLContentSink::ProcessSCRIPTTag() it says:

 // Don't process scripts that aren't JavaScript and don't process
 // scripts that are inside iframes, noframe, or noscript tags,
 // or if the script context has script evaluation disabled:

But we would go ahead and try to process scripts even when script was disabled
because nsIScriptContext->GetScriptsEnabled is always returning true (which
seems bad in and of itself?)  Perhaps I should leave the check of
nsIScriptContext->GetScriptsEnabled there as well - and if either that or
IsScriptEnabled is false, don't process the script tag?

Comment 3

[Marek Z. Jeziorek]

TM to 0.9.2 (as a bug that needs to be done before RTM) per PDT triage.

Comment 4

[Eric Pollmann]

Attachment: updated patch

Comment 5

[Eric Pollmann]

Harish, Johnny, can you r=/sr= this patch?  Thanks!

Comment 6

[Eric Pollmann]

Johnny says "sr=[e-mail]", thanks!

Comment 7

[harishd]

Nit pick: Could you delay JSContext declaration further? ;-)

r=harishd.

Comment 8

[Eric Pollmann]

Thanks - in fact, Johnny suggested the exact same change during super-review, so
I have declared it right where it is assigned:

+  JSContext* cx = (JSContext *) scriptContext->GetNativeContext();
+  NS_ENSURE_TRUE(cx, PR_TRUE);

Handing to drivers for approval...

Comment 9

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to the trunk.
(on behalf of drivers)

Comment 10

[Eric Pollmann]

Patch checked in.

There is no way to directly verify this bug at the present time.  As embedding
clients begin to have features like disabling javascript in a mail window, and
so forth, this should be verifiable.

Comment 11

[Chris Petersen]

Verifying per last comments.