Closed
Bug 280120
Opened 20 years ago
Closed 20 years ago
Turning off js in help doesn't work properly
Categories
(SeaMonkey :: Help Documentation, defect)
SeaMonkey
Help Documentation
Tracking
(Not tracked)
RESOLVED
FIXED
seamonkey1.0beta
People
(Reporter: stefanh, Assigned: bzbarsky)
Details
Attachments
(3 files)
1.52 KB,
patch
|
Details | Diff | Splinter Review | |
2.12 KB,
patch
|
Details | Diff | Splinter Review | |
3.93 KB,
patch
|
peterv
:
review+
neil
:
superreview+
|
Details | Diff | Splinter Review |
Turning off Javascript in Help by adding the line
helpBrowser.docShell.allowJavascript = false; in help.js doesn't seem to work
properly.
It works if Javascript is enabled in Preferences --> Advanced --> Script &
Plug-ins. But turning off Javascript in preferences seem to enable it in Help.
How to repro:
1. Turn off js in Help by adding helpBrowser.docShell.allowJavascript = false;
in help.js.
2. Add a script in the welcome page (welcome.xhtml)
3. Start Mozilla and turn off js in preferences (Preferences --> Advanced -->
Script & Plug-ins)
4. Notice that the script works.
Reporter | ||
Comment 1•20 years ago
|
||
Here's a patch that illustrates the problem:
Apply the patch, turn off js in preferences and open Help (Help Contents in
Help menu)
Comment 2•20 years ago
|
||
CCing people who might have a clue about how this works (or doesn't...)
Assignee | ||
Comment 3•20 years ago
|
||
Looks like if JS is disabled the security manager special-cases non-system
chrome: uris and doesn't check with the docshell (see
nsScriptSecurityManager::CanExecuteScripts).
That's probably wrong, but I'm not sure what the right behavior is, exactly.
Should docshells be able to _enable_ script if it's disabled globally and they
explicitly set the flag to true, say?
Comment 4•20 years ago
|
||
The comment refers to about: URLs, but they aren't tested for. What the code
actually does is override the docShell setting for chrome: URLs but only when
javascript is globally disabled. Do we actually need that special case? Or do
we still want it, but for about: URLs rather than chrome: URLs?
Assignee | ||
Comment 5•20 years ago
|
||
about: URLs redirect to chrome, so this code used to be testing for them. But
now that chrome: ends up using originalURI, I think this test could be safely
switched to about: instead of chrome:.
Comment 6•20 years ago
|
||
As it stands that will break Editor because it currently uses about:blank for a
new page and that the code block in question overrides the docShell setting.
The only benefit that I can see of allowing about: pages to run scripts is for
about: itself so that it can show the UA when JavaScript is disabled.
Assignee | ||
Comment 7•20 years ago
|
||
At least that, yes. I guess the other about: pages that run JS have system
principals.... Is about: broken right now, btw, with JS disabled?
Reporter | ||
Comment 8•20 years ago
|
||
(In reply to comment #7)
> Is about: broken right now, btw, with JS disabled?
Yes
Assignee | ||
Comment 9•20 years ago
|
||
So either we need to special-case just about: (URL, not scheme) here, or we need
to come up with a more flexible setup for this stuff for about: (scheme) urls.
Assignee | ||
Updated•20 years ago
|
Flags: blocking1.8b?
Updated•20 years ago
|
Flags: blocking1.8b? → blocking1.8b+
Assignee | ||
Comment 11•20 years ago
|
||
This is a really simple bug to fix; we just need to decide what approach to
take. Note that not only is help broken, so is "about:"....
sicking, peterv, any ideas?
Bumping severity to major, since this is effectively breaking our security model
in some cases.
Severity: minor → major
Assignee | ||
Comment 12•20 years ago
|
||
So to be precise, we want to:
1) Not mess with whether JS is enabled in chrome
2) Enable JS in "about:" (only, not other URIs with that scheme, especially not
about:blank).
If someone edits "about:" that will give the editor conniptions, but fixing that
would require some new api to communicate "this document should ignore JS
disabling".
Assignee | ||
Comment 13•20 years ago
|
||
Attachment #174486 -
Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #174486 -
Flags: review?(peterv)
Comment 14•20 years ago
|
||
Comment on attachment 174486 [details] [diff] [review]
Proposed patch
We can worry about that when editor learns xhtml.
Attachment #174486 -
Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
Assignee | ||
Comment 15•20 years ago
|
||
Actually... biesi, would this break error pages?
If so, perhaps the right answer is to keep the chrome check (in addition to the
"about" check), and just put them both where I put the "about" check, after we
check for the docshell locking out script.
Of course that will cause error pages to not work in docshells that disable
script....
Comment 16•20 years ago
|
||
hm, yes, it seems it would :-/
maybe error pages need an own scheme...
Assignee | ||
Comment 17•20 years ago
|
||
Either that, or we need some other way of telling which principals shouldn't
have script disabling apply to them...
Comment 18•20 years ago
|
||
(In reply to comment #15)
>Actually... biesi, would this break error pages?
By my limited understanding error pages currently have chrome:// content URLs
and I therefore assume system principal which is already specially cased.
Assignee | ||
Comment 19•20 years ago
|
||
That's the question... do error pages have system principals?
Updated•20 years ago
|
Attachment #174486 -
Flags: review?(peterv) → review+
Assignee | ||
Updated•20 years ago
|
Assignee: neil.parkwaycc.co.uk → bzbarsky
Target Milestone: --- → mozilla1.8beta2
Assignee | ||
Updated•20 years ago
|
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Updated•20 years ago
|
Target Milestone: mozilla1.8beta2 → ---
Assignee | ||
Updated•20 years ago
|
Target Milestone: --- → Seamonkey1.0beta
You need to log in
before you can comment on or make changes to this bug.
Description
•