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

VERIFIED FIXED in mozilla0.9.2

Status

()

Core
Layout
VERIFIED FIXED
17 years ago
17 years ago

People

(Reporter: Eric Pollmann, Assigned: Eric Pollmann)

Tracking

Trunk
mozilla0.9.2
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fix in hand, need approval, URL)

Attachments

(2 attachments)

(Assignee)

Description

17 years ago
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
(Assignee)

Comment 1

17 years ago
Created attachment 32247 [details] [diff] [review]
patch, check security manager rather than prefs + cleanup
(Assignee)

Comment 2

17 years ago
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?
Status: NEW → ASSIGNED

Updated

17 years ago
Target Milestone: --- → mozilla0.9.1

Comment 3

17 years ago
TM to 0.9.2 (as a bug that needs to be done before RTM) per PDT triage.
Target Milestone: mozilla0.9.1 → mozilla0.9.2
(Assignee)

Comment 4

17 years ago
Created attachment 38198 [details] [diff] [review]
updated patch
(Assignee)

Updated

17 years ago
Whiteboard: fix in hand
(Assignee)

Comment 5

17 years ago
Harish, Johnny, can you r=/sr= this patch?  Thanks!
Whiteboard: fix in hand → fix in hand, need r/sr/a
(Assignee)

Comment 6

17 years ago
Johnny says "sr=jst@netscape.com", thanks!
Whiteboard: fix in hand, need r/sr/a → fix in hand, need r/a

Comment 7

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

r=harishd.
(Assignee)

Comment 8

17 years ago
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...
Whiteboard: fix in hand, need r/a → fix in hand, need approval

Comment 9

17 years ago
a= asa@mozilla.org for checkin to the trunk.
(on behalf of drivers)
Blocks: 83989
(Assignee)

Comment 10

17 years ago
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 11

17 years ago
Verifying per last comments.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.