Last Comment Bug 32372 - should be able to disable CSS (Use Style > None or global pref)
: should be able to disable CSS (Use Style > None or global pref)
Status: RESOLVED FIXED
[WAI-P1][HTML4-14.3.1][CSS1-7] se-rad...
: css1, helpwanted, html4, relnote
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: All All
: P2 normal with 12 votes (vote)
: mozilla1.8alpha3
Assigned To: fantasai
: Hixie (not reading bugmail)
Mentors:
http://www.opensa.org/
: 34825 35322 51690 55989 93241 107326 127464 180649 215803 233471 241139 264859 (view as bug list)
Depends on: 51688
Blocks: html4.01 68416 useragent altss 318175 51690 104166 253332
  Show dependency treegraph
 
Reported: 2000-03-18 08:01 PST by Martin Horwath
Modified: 2014-04-26 03:07 PDT (History)
54 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
IRC discussion about how to implement this, and other issues (5.08 KB, text/plain)
2002-01-09 12:29 PST, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details
patch v0.1 (17.88 KB, patch)
2002-02-05 20:34 PST, fantasai
no flags Details | Diff | Review
Firefox Theme widget (25.03 KB, image/png)
2004-04-20 19:27 PDT, Dave Miller [:justdave] (justdave@bugzilla.org)
no flags Details
patch (25.63 KB, patch)
2004-07-10 12:38 PDT, fantasai
fantasai.bugs: review-
Details | Diff | Review
patch (34.51 KB, patch)
2004-07-18 14:14 PDT, fantasai
no flags Details | Diff | Review
patch (36.14 KB, patch)
2004-08-01 08:07 PDT, fantasai
fantasai.bugs: review-
fantasai.bugs: superreview-
Details | Diff | Review
patch (36.55 KB, patch)
2004-08-01 15:39 PDT, fantasai
fantasai.bugs: review-
Details | Diff | Review
patch (36.52 KB, patch)
2004-08-01 16:16 PDT, fantasai
neil: review+
dbaron: superreview+
Details | Diff | Review
final patch (36.02 KB, patch)
2004-08-04 14:14 PDT, fantasai
no flags Details | Diff | Review

Description Martin Horwath 2000-03-18 08:01:06 PST
in Preferences/Advanced/enable style sheets has no effect.

i thought this option should disable any styles on any page, but
it simply does nothing.

something like this,

***
  <link href="/opensa.css" rel="stylesheet" type="text/css">

***
<style>
  body { font-size: 50px }
</style>

***
 if (document.getElementById) {
   document.write('<link rel=STYLESHEET type="text/css"
href="/layout/css/gecko.css">');

should be simply ignored.

testet on win98 build 2000031708
Comment 1 Chris McKinley 2000-05-05 12:56:19 PDT
Turning off CSS in the Prefs menu actually does nothing.  I turned it off and 
went to the following page: http://www.lehigh.edu/~cjm3/links.html, where the 
following CSS code is used:

<style>
<!--
a{text-decoration:none}
//-->
</style>

After reloading the page (after turning of CSS), the links still were not 
underlined.  I even restarted Mozilla and had the same results.

Build ID: 2000050508
Operating System: Win98, Intel Pentium II
Comment 2 Gervase Markham [:gerv] 2000-05-11 17:45:03 PDT
Confirming.

Gerv
Comment 3 don 2000-05-11 23:28:34 PDT
M17 ...
Comment 4 leger 2000-05-12 12:57:19 PDT
ekrock, is it impt to be able to turn off for beta2.
Comment 5 Gervase Markham [:gerv] 2000-05-12 15:56:32 PDT
*** Bug 34825 has been marked as a duplicate of this bug. ***
Comment 6 ekrock's old account (dead) 2000-05-13 13:11:47 PDT
David, Ian--does the CSS or HTML spec *require* that applications have a 
feature/pref enabling style sheets to be turned off?

Pierre, Mark--how much work is it in the code to just turn off CSS?

Possible reasons to mark nsbeta2+:
1) strong reason: if mandated by a W3C spec
2) another good reason: there may be some users with visual impairments such as 
colorblindness who find that a particular CSS-specified page color scheme makes 
it hard/impossible for them to read a particular page; for those users, the 
ability to turn off CSS would improve accessibility
3) weaker reason: backward compatibility of "degree of functionality control" 
with Nav4

I *suspect* we can enable this easily but need more info from our standards 
gurus about requirements and from engineering about LOE.
Comment 7 Hixie (not reading bugmail) 2000-05-15 02:42:01 PDT
HTML 4.01, section 14.3.1, paragraph 7:
#  User agents SHOULD also allow users to disable the author's style 
#  sheets entirely, in which case the user agent MUST not apply any
#  persistent or alternate style sheets.
Comment 8 Marc Attinasi 2000-05-15 13:59:42 PDT
From Ian's post opf the HTML spec it looks like it is optional (noting the 
SHOULD). We cannot just turn off CSS - nothing would work if we did that. We 
can, however, disable the author's stylesheet if we can identify it. The 
underlying support is there in the StyleSet and SyleSheet, however we need to 
hook up some plumbing to get the correct sheet(s) and disable them, and also to 
re-enable them. Probably not a huge amount of work (a few days?), but it doesn't 
look like it is essential. 

Note that for bug 38026 I need to implement something similar in the 
StyleSet (ability to enable/disable a QuirkMode style sheet) so I can generalize 
the implementation to support something like this (enable/disable *any* 
stylesheet, including author style sheets). Once that work is done, the 
preferences can make calls into the StyleSet to do the work.
Comment 9 Peter Trudelle 2000-05-15 14:09:15 PDT
[nsbeta2-] Make sure this has to do with link style sheets rather than default.
Comment 10 ekrock's old account (dead) 2000-05-16 00:02:09 PDT
<tough_decision>Sigh. All right. (1) It's a SHOULD, not a MUST, in the HTML 4.01 
spec. (2) IE5 Win doesn't seem to have this either. Marking FUTURE, helpwanted, 
and relnote as Mark clearly indicates this is a nontrivial enhancement, and 
we're out of time for enhancements. Unless I'm persuaded otherwise in the next 
few days, I'll file a bug to have the "disable style sheets" pref removed from 
the UI. </tough_decision>
Comment 11 Jerry Baker 2000-05-16 00:37:17 PDT
> We cannot just turn off CSS - nothing would work if we did that. We 
> can, however, disable the author's stylesheet if we can identify it.

Isn't that exactly what "...in which case the user agent MUST not apply any 
persistent or alternate style sheets" is addressing. It seems to me that this 
says, "if you have the option of turning off the author's style sheets, then you 
must not use another in its place".
Comment 12 Marc Attinasi 2000-05-16 06:28:21 PDT
jerrybaker, what I meant was that we must still apply the *user agent* style 
rules (in html.css and xul.css), so we cannot 'turn off style'. You are correct, 
however, in that we MUST not apply any author stylesheets, persistent or 
altermate, when we do implement thsi in the future.
Comment 13 Chris McAfee 2000-05-16 16:36:03 PDT
over to ekrock to resummarize and/or this bug.
I can do pref UI stuff, not clear what's going on here.
Comment 14 sairuh (rarely reading bugmail) 2000-05-16 16:41:45 PDT
until we get help on this feature, should the Pref be pulled from the UI?
Comment 15 ekrock's old account (dead) 2000-05-16 21:45:07 PDT
To clarify:
1) retitling this bug "should be able to enable/disable style sheets via a pref 
in UI", leaving as FUTURE, reassigning to attinasi
2) opening new, separate bug 39552 on mcafee to eliminate the enable/disable 
pref of the UI for nsbeta2 (and FCS)
Comment 16 Pierre Saslawsky 2000-05-25 19:17:28 PDT
*** Bug 35322 has been marked as a duplicate of this bug. ***
Comment 17 Karl Ove Hufthammer 2000-05-31 11:28:15 PDT
Here's a quote from the W3C User Agent Accessibility Guidelines, checkpoint 4.12:

"Allow the user to select from available author and user style sheets or to
ignore them. *[Priority 1] *"

This is a priority 1 checkpoint:

"This checkpoint *must* be satisfied by user agents, otherwise one or more
groups of users with disabilities will find it impossible to access the Web.
Satisfying this checkpoint is a basic requirement for enabling some people
to access the Web."

There should be a way to disable (document) style sheets in Mozilla, preferably as a button or a menu-item. Perhaps a small button in the statusbar for disabling/enabling style sheets?
Comment 18 Antti Näyhä 2000-06-14 00:01:28 PDT
IMHO, it would be logical to include the disable option in the "Use Stylesheet" 
menu.  For details, see http://www.people.fas.harvard.edu/~dbaron/css/ssui/ and 
the discussion at bug 6782.
Comment 19 sairuh (rarely reading bugmail) 2000-08-30 22:07:20 PDT
spam: adding mostfreq, not necessarily due to many dups, but because i run into 
this problem frequently (thus want to get it my queries easily).
Comment 20 Pierre Saslawsky 2000-08-31 00:42:16 PDT
Disable the author stylesheets? That's what XBL style bindings are for...
Geek humor aside, Marc, this should be fairly easy to do if you can find a way to 
tell whether the PresContext (or whatever owning object we're in) belongs to the 
chrome or to the content (because we want to do this in the content area only, 
right?). Looking for "isChrome" in nsDocumentViewer.cpp or for "isContent" in 
nsFrameFrame.cpp, or just asking hyatt, should put you on the right track. If you 
find the trick before me, please drop a note in bug 31816.

I think that this pref should be active only in HTML documents.
Comment 21 Hixie (not reading bugmail) 2000-08-31 14:21:14 PDT
And XHTML documents. And MathML documents. And XUL documents. And documents that
only have elements from those namespaces.

I think it might be easier just to always have this pref available...
Comment 22 Marc Attinasi 2000-09-05 17:11:34 PDT
From Daivd Hyatt:

ben says you need to know how to tell if you're in chrome... 

This info is on the docshell... you can see an example of the check in 
nsDocumentViewer.cpp where I check to see whether or not I want to use chrome or 
content user stylesheets.... 

  nsCOMPtr<nsIDocShellTreeItem> docShell = (get it from whereever you are...) 
  PRInt32 shellType; 
  docShell->GetItemType(&shellType); 
  PRBool isChrome = (shellType == nsIDocShellTreeItem::typeChrome); 
Comment 23 Tobias Burnus 2000-09-07 03:28:59 PDT
I filled in bug 51690 (disable stylesheets (also style="" and <style>) via
View|Use stylesheet).
This brought me to the question:
Does disable means that "Use stylesheet" is disabled or that we default to
"None" if we load a page so the user can later use this menu to choose a style?
Comment 24 Gervase Markham [:gerv] 2000-09-10 04:48:22 PDT
Removing mostfreq - this bug does not qualify for this keyword under the 
established criteria.

Gerv
Comment 25 sairuh (rarely reading bugmail) 2000-09-11 13:02:16 PDT
adding se-radar to status so that i can find this bug more easily. pls don't
remove it [yet], thx.
Comment 26 Boris Zbarsky [:bz] 2000-10-10 16:04:29 PDT
*** Bug 55989 has been marked as a duplicate of this bug. ***
Comment 27 Matthew Tuck [:CodeMachine] 2000-10-12 23:37:51 PDT
I think it would be better to make this a main part of the stylesheet UI and
remember the setting between windows and invocations.
Comment 28 Hixie (not reading bugmail) 2000-10-24 14:53:54 PDT
Closer examination of the CSS1 specification indicates that this is a CSS1
full-support requirement too. (Section 7.)

RELEASE NOTE ITEM:
   In this release it is not possible to totally disable author stylesheets.
   It is possible to use user stylesheets (see XXX) to achieve more fine-grained
   control over the styling of web pages, however.
Comment 29 Marc Attinasi 2000-10-24 15:26:08 PDT
I'm not sure what you mean by full-support, Ian, but in section 7 I read:

This specification also recommends, but doesn't require, that a UA:
  - allows the reader to specify personal style sheets
  - allows individual style sheets to be turned on and off

as indicating that it is optional, but recommended. By full-support do you mean
that we implement all _recommendations_ as well as _requirements_?

I agree that the release note item is a good idea, thanks for writing it.
Comment 30 Hixie (not reading bugmail) 2000-10-24 16:06:48 PDT
Marc: Yes, full support as distinct from conformant support.
Comment 31 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2000-11-19 17:26:42 PST
Nominating for mozilla 0.9.  There have been a number of strongly negative
comments about Netscape 6 on c.i.w.a.s because of the lack of this feature.

Probably the safer way to implement this pref would be around the loading of
stylesheets and parsing of style elements/attributes.  However, this would still
allow HTML presentational hints.  Implementing this at the cascade level would
also block those, I think (if we've implemented them correctly, although I
suspect we may not in some cases).  Would there be any unexpected consequences
of doing that?  Would we want 2 separate prefs?
Comment 32 Katsuhiko Momoi 2000-11-20 14:20:47 PST
CC'ed erik & momoi.
Comment 33 Marc Attinasi 2000-11-20 15:15:20 PST
I'm thinking that we want to turn on or off individual stylesheets more than
having a global 'use stylesheets' preference. That is what CSS1 recommends.
Having a global pref could simply be a shortcut for always disabling all
stylesheets, of course, but we would still benefit from a UI to toggle each
stylesheet individually (similar to the Alternate Stylesheets UI?)
Comment 34 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2000-11-20 16:12:43 PST
I don't see any reason for toggling stylesheets individually other than as
described by the author with TITLEs.  Allowing that discourages or encourages
authors from using linked stylesheets versus STYLE elements for reasons other
than the ones on which they should be basing that decision.  It also discourages
authors from writing modular stylesheets, since the author would have to worry
about the effects of disabling part of the stylesheets but not all.
Comment 35 Marc Attinasi 2000-11-27 16:33:40 PST
One reason to support disabling of individual stylesheet is the CSS1
specification. Section 7 says:

This specification also recommends, but doesn't require, that a UA:
 - allows the reader to specify personal style sheets
 - allows individual style sheets to be turned on and off

I guess we could argue that the spec is wrong, and I cannot find the same
recommendation in the CSS2 specification. I don't personally care either way, so
hopefully Eric can tell us what he thinks the product should support -
enabling/disabling of all stylesheets, or per-stylesheet.

Also, do we really want to suppress HTML presentational hints along with the
stylesheets? I do not see why we would since from an author's perspective they
are NOT stylesheets (even though we may implement some of them as style rules
internally).
Comment 36 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2000-11-27 16:45:02 PST
The argument for including nonpresentational hints in the "turn all stylesheets
off" option is that a user who chooses to turn off all stylesheets (including
persistent ones) probably can't use the page with the styles given, which may or
may not be specified using stylesheets.
Comment 37 Marc Attinasi 2000-11-27 16:57:44 PST
Agreed. Then what we really want is an option to 'turn off all style'. I doubt
users want to be bothered with figuring out what parts of the page are
implemented with stylesheets vs. HTML attributes and elements. So what then
would be included in the presentational hints that we disable? Font sizes, font
faces, font weight, underline, bold, italics, color, and background? Hey, why
not just make a 'show as plain text' option! Seriously, that is pretty much what
it would be, except that you would have images too, and borders.
Comment 38 ekrock's old account (dead) 2000-11-30 16:37:29 PST
A *per-stylesheet* disabling UI would be incredibly confusing and utterly
useless for ordinary users. A classic "provide too much UI and complexity for
the user to understand, thus providing no benefit"-type mistake. (Yes, I'm sure
that a few developers might find such a beast handy for testing & QA during page
development, but Mozilla/N6 are a browser application, not a sophisticated style
sheet development tool. Let's leave authoring tool functionality to the
authoring tool vendors.) A single "turn off style sheets" option is clearly the
way to go.

As for turning off presentational hints at the same time: let's be careful about
reinterpreting what the standards say. First of all, by presentational hints,
are you referring to hardcoded FONT FACE tags and the like? If so, we'd be in
violation of the HTML spec if we ignored those tags as well when the user turned
of style sheets, yes? After all, we'd be ignoring markup that, though
deprecated, should be respected and is used by developers as a fallback for
non-stylesheet browsers (and thus arguably should be preserved if style sheets
are turned off in a style sheet-capable browser).

My take:
1) the ability to turn on/off all style sheets (binary, global) is something
that Netscape should invest its time in.
2) The ability to ignore presentational hints is a separate feature, an
enhancement request, and honestly not a good use of Netscape resources at this
time.

My proposal:
a) Let's have Marc limit himself to working on a binary "turn off style sheets"
checkbox (or View menu item, or whatever--the point is it should be an
all-or-nothing setting that turns off the application of CSS only)
b) A separate enhancement request bug on the "turn off presentational hints"
should be opened and reassigned to nobody@mozilla.org until someone not at
Netscape takes this work on.

Netscape developers should remain focused on stability, performance, and bug
fixes for some time to come, not work on enhancements that will only be use to a
tiny fraction of our user base if that.
Comment 39 Erik van der Poel 2000-11-30 16:46:05 PST
From an accessibility standpoint, it might be useful to be able to switch off
all style sheets except for the user style sheet.
Comment 40 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2000-11-30 17:01:09 PST
I disagree with Eric Krock's comments above.  Turning off all stylesheets is
only a useful feature in that it allows the user to control the browser when the
page author misbehaves.  If it only works for style specified in CSS, it won't
be very useful.  Turning off the FONT tag does *not* break compliance with HTML 4.0.

I also suspect that, from an implementation standpoint, turning off all author
style is easier than turning off all CSS, since it can be implemented easily at
the cascade level.  Turning off all CSS (including STYLE elements and
attributes) seems a bit more difficult.
Comment 41 Gervase Markham [:gerv] 2001-01-23 15:15:02 PST
What's the status here? :-) Marc, is this very much on your back burner, or are 
we going to get it for 0.9 or 1.0?

Gerv
Comment 42 Marc Attinasi 2001-01-23 15:21:53 PST
Still a back-burner issue, but my manager has just informed me that I need to
start addressing the nominations... That should happen over the next few days.
Comment 43 HJ 2001-01-27 18:08:52 PST
Can't you filter external stylesheet out on HTTP level?
Comment 44 Gervase Markham [:gerv] 2001-03-28 05:36:57 PST
Add more keywords. We really _do_ need to do this for 1.0, and that means doing 
it for 0.9 if it's going to get any serious testing. 

Gerv
Comment 45 David Hallowell 2001-08-02 09:43:27 PDT
*** Bug 93241 has been marked as a duplicate of this bug. ***
Comment 46 David Hallowell 2001-08-02 09:46:13 PDT
Adding CSS to the summary, may help people searching for this bug if they're
looking for "CSS" rather than "style sheets"
Comment 47 Christopher Hoess (gone) 2001-10-29 07:34:09 PST
*** Bug 107326 has been marked as a duplicate of this bug. ***
Comment 48 Pierre Saslawsky 2001-11-05 02:17:20 PST
Taking this style bug off Marc's list.
Comment 49 Pierre Saslawsky 2001-11-05 02:17:53 PST
*** Bug 51688 has been marked as a duplicate of this bug. ***
Comment 50 Pierre Saslawsky 2001-11-18 20:57:22 PST
*** Bug 51690 has been marked as a duplicate of this bug. ***
Comment 51 Jeremy M. Dolan 2001-12-03 21:54:15 PST
Rather then a pref, which would be quite inaccessible when you simply want to
disable style temporarily, this should be integrated into the current
view->stylesheets menu.

As far as prefs, the current ones that disable custom fonts/colors etc should
simply apply to CSS as well as the traditional HTML <font> method, as users
using those options are most likely doing so for permanent accessibility
reasons, and don't really care if the moron web designer used CSS or <font> to
set the page to be 6pt green text on a blue background.

How about this for the menu:

 None       # No stylesheets applied
 -------    # divider
 Default    # Only persistant styles applied. Equivilent to current 'none'
 funky      # Prefered sheet listed next, which would be selected at page load
 clean      # Alternate stylesheets listed next, in order of referance on the
 very red   #   page (unless the spec says they should be listed in some other
            #   order)

Currently, when only a persistant stylesheet is active, 'None' is checked in the
menu. As shown above, I suggest 'Default' be added, which is only the persistant
styles applied.

None actually meaning, 'no stylesheets', is alot more obvious than it meaning
'no alternate/prefered stylesheets', especially to people who haven't read the
CSS and HTML specs.
Comment 52 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2001-12-04 15:51:46 PST
> Default    # Only persistant styles applied. Equivilent to current 'none'

It doesn't make sense to call that "Default", since it's not the default.  I
wrote a proposal in the main alternate stylesheets UI bug a year or two ago with
suggestions for how to do this.
Comment 53 fantasai 2001-12-28 14:33:51 PST
Some history: It was decided in bug 6782 to have a no-author-level-styles option
in the Use Style menu; this was filed as bug 51690, which was marked as a
duplicate of this bug. Bug 51688 was filed to deal with the current "none"
option--initially to change the wording until 51690 could be fixed.
Comment 54 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2002-01-09 12:29:49 PST
Created attachment 64169 [details]
IRC discussion about how to implement this, and other issues
Comment 55 Kevin McCluskey (gone) 2002-02-05 15:19:45 PST
Moving to Mozilla1.1. Engineers are overloaded with higher priority bugs.
Comment 56 fantasai 2002-02-05 20:34:31 PST
Created attachment 68076 [details] [diff] [review]
patch v0.1

This unfinished fix enables/disables author level styles.
There are two outstanding issues:
  1.) The refresh function needs to be called on *every* page load.
      I don't know how to do this.
  2.) Switching around with CSS generated content images has weird results and
      sometimes crashes. Reproducible. Might have something to do with the
      DeviceContext calls made in SetPreferenceStyleRules that I'm not making,
      or something else I'm not yet aware of. I'm going to look into this;
      hopefully I'll find the problem.

Also, I haven't tested it with framesets yet.
Comment 57 Kevin McCluskey (gone) 2002-02-22 09:51:20 PST
Bulk moving from Moz1.1 to future-P1. I will pull from this list when scheduling
work post Mozilla1.0.
Comment 58 Boris Zbarsky [:bz] 2002-02-23 13:57:08 PST
*** Bug 127464 has been marked as a duplicate of this bug. ***
Comment 59 Jerry Baker 2002-05-27 14:32:46 PDT
Mass removing self from CC list.
Comment 60 Jerry Baker 2002-05-27 14:58:05 PDT
Now I feel sumb because I have to add back. Sorry for the spam.
Comment 61 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2002-06-19 21:16:57 PDT
Assigning pierre's remaining Style System-related bugs to myself.
Comment 62 Tim Powell 2002-08-19 14:42:14 PDT
I notice that style sheet toggling can be done per page via a bookmarklet:
http://www.ninjakitten.net/digiboy/2002_06.php#post000262
Seems to work fine.
Comment 63 Hixie (not reading bugmail) 2002-09-13 17:28:37 PDT
The bookmarklet doesn't disable non-CSS style hints.
Comment 64 Boris Zbarsky [:bz] 2002-11-17 16:08:04 PST
*** Bug 180649 has been marked as a duplicate of this bug. ***
Comment 65 Rafael Ebron (:rebron) 2002-12-21 09:24:01 PST
cc:ing Aaron 

Is this another accessibility win?  

-R
Comment 66 Aaron Leventhal 2002-12-23 09:44:38 PST
Sure, this would be nice for some visually impaired users if it was something
that could be easily toggled. I'd like to see something like this on the pref
toolbar from xul planet. Not a high priority compared to our other accessibility
work, at least at the moment.
Comment 67 Stewart Gordon 2003-03-14 10:28:51 PST
There are really two separate issues here:

(a) being able to disable CSS (or for those of you who prefer to say it this
way, use only the UA stylesheet) as a global user preference
(b) being able to switch the style sheet off temporarily (View -> Use Style ->
None or something) for such purposes as testing or reading badly-written pages.

(a) could merely set the default, while still allowing the user to override it
either way from the Use Style submenu.
Comment 68 fantasai 2003-03-14 19:44:22 PST
I think having two switches, one global and one "local" will just confuse the
user. Are they two interfaces to the same switch? Are they two separate
settings--e.g. does one override the other? Which one? It's better, IMO,
to have just one switch in the Use Style menu and remember its last setting
between sessions. The disabling author style, btw, will be quite drastic, as it
will not only disable all CSS, but most HTML formatting as well. I doubt its use
as a global preference will be very common; forcing a color or font override is
usually more useful.
Comment 69 Stewart Gordon 2003-03-17 04:03:09 PST
Please clarify:

1. If the global setting overrode the local setting, what would the local setting do at all?
2. How would disabling the author style sheet disable HTML formatting?
Comment 70 fantasai 2003-03-17 20:06:21 PST
> 1. If the global setting overrode the local setting, what would the local
> setting do at all?

"Obviously" the local setting overrides the global one, but you have to convey
this model to the users, which is easier said than done. They don't need a
separate global setting; why confuse them with one?

> 2. How would disabling the author style sheet disable HTML formatting?

Disabling author styles disables HTML formatting because HTML formatting is
part of the author style level[1]. It is style assigned by the author, so it
certainly belongs there.

From a UE perspective, we should not make a distinction between <font
color="#FF0000"> and <span style="color: #FF0000"> because the user doesn't
care. Lime text on a Green background is hard to read whether it's specified
with HTML attributes or CSS properties.

[1] http://www.w3.org/TR/REC-CSS2/cascade.html#cascade , specifically 6.4.1 and
    6.4.4
Comment 71 Stewart Gordon 2003-03-18 02:37:08 PST
Your link gives this line:

"It is therefore important that the user agent give the user the ability to turn off the 
influence of a certain style sheet, e.g., through a pull-down menu."

But 6.4.4 seems rather odd.  Why should HTML presentation hints be at the beginning of 
the author style sheet, rather than treated as being in style="..." attributes?  And can't a 
makeshift "author style sheet" be used to handle HTML presentation hints if the real 
author style sheet is disabled?

As for your "UE perspective", I'm not totally sure what to say.  But what is for certain is that 
there is sometimes the need to disable an author style sheet in order to read a badly 
coded page.
Comment 72 fantasai 2003-03-18 08:40:55 PST
> Why should HTML presentation hints be at the beginning of 
> the author style sheet, rather than treated as being in style="..." 
> attributes? 

So that author CSS can override HTML presentation hints. If I make links silver
with <body link="silver">, I should be able to make some of them aqua with
  .navigationbar :link {color: aqua}

> And can't a makeshift "author style sheet" be used to handle HTML presentation
> hints if the real author style sheet is disabled?

Technically, yes. But we don't want to do that because, as you say, 

"there is sometimes the need to disable an author style sheet in order to read a
 badly coded page."

And there is no less a need to disable <font> and other presentation hints in
order to read a page badly coded with HTML presentational hints. You *do not
care* if a poor color combination was specified in HTML or CSS. You just want to
disable it.
Comment 73 Stewart Gordon 2003-03-18 09:08:48 PST
You misunderstand.  I was actually thinking of bad CSS and JS rather than bad
HTML.  There are situations where a page is inaccessible, e.g. because it has
used some combination of CSS and JS to show/hide content, and the code is
somewhere broken or otherwise heavily reliant on having the right browser.

Witness http://www.sineadquinn.tv for example.  OK, so this one would need a bit
more than just switching off the author style sheet, but surely some sites would
work if only explicit CSS declarations were ignored even if HTML presentation
hints are allowed to shine through.

That's what happened on browsers that predated CSS, didn't it?
Comment 74 fantasai 2003-03-18 09:51:45 PST
> surely some sites would work if only explicit CSS declarations were ignored even
> if HTML presentation hints are allowed to shine through.

Certainly, but why give HTML presentation hints (which are deprecated, btw) such
a stronger precedence than CSS? I argue that if the same site used CSS instead of
HTML presentation hints, I should get the same result if I turn off author style.

And you still have this whole problem with color contrast, etc, which should be
handled the same in both HTML and CSS.
Comment 75 Stanimir Stamenkov 2003-03-19 00:31:52 PST
A side comment (to the last few): Should the DOM Style interfaces be disabled
when the style is "switched off"? Or may be made read-only to not be modifiable
by scripts.
Comment 76 Boris Zbarsky [:bz] 2003-03-19 00:48:29 PST
DOM style is just inline style.  If inline style is off, so is DOM style (it can
be changed, but has no effect on rendering).
Comment 77 Stewart Gordon 2003-05-06 02:31:43 PDT
Re: fantasai

Yes, but if on a given page it's only the CSS that's broken and not the HTML, why should 
the luser be forced to disable both?
Comment 78 Stewart Gordon 2003-05-06 02:34:14 PDT
Re: famtasai

Yes, but if on a given page it's only the CSS that's broken and not the HTML, why should 
the luser be forced to disable both?

HTML presentation hints in a CSS-formatted page are, after all, supposed to be for 
graceful degradation.
Comment 79 Hixie (not reading bugmail) 2003-05-06 03:47:14 PDT
What if it is the HTML that is broken and not the CSS? What if one stylesheet of
the two linked to a page is fine, but the second is not? In fact, what if one
third of the fifth stylesheet is the problem, but the rest is not?

And how should the user discover this?

As far as the overwhelming majority of users are concerned, there's no
difference between HTML hints and CSS hints.
Comment 80 Stewart Gordon 2003-05-06 04:07:52 PDT
> What if it is the HTML that is broken and not the CSS?

Good question.  But can you supply an example of such a page?

> What if one stylesheet of the two linked to a page is fine, but the
> second is not?  In fact, what if one third of the fifth stylesheet
> is the problem, but the rest is not?

That would be a tricky question.  But how likely is it that, should
that be the case, using no stylesheet at all and letting the HTML
hints shine through wouldn't GD?

> And how should the user discover this?

That depends.  But discovering the case I mentioned above would be
perfectly straightforward given the option.

> As far as the overwhelming majority of users are concerned, there's
> no difference between HTML hints and CSS hints.

I expect that the overwhelming majority of users also don't care
about the difference between alternative stylesheet 1 and alternative
stylesheet 2.  So why shouldn't we give the handful who either are
interested or need it the option of using no stylesheet at all?
Comment 81 Hixie (not reading bugmail) 2003-05-06 04:28:46 PDT
>> What if it is the HTML that is broken and not the CSS?
>
> Good question.  But can you supply an example of such a page?

That's trivial. A page with <font size="1"> at the top.


>> What if one stylesheet of the two linked to a page is fine, but the
>> second is not?  In fact, what if one third of the fifth stylesheet
>> is the problem, but the rest is not?
>
> That would be a tricky question.  But how likely is it that, should
> that be the case, using no stylesheet at all and letting the HTML
> hints shine through wouldn't GD?

GD?


>> And how should the user discover this?
>
> That depends.  But discovering the case I mentioned above would be
> perfectly straightforward given the option.

So you want the user to try various options until they think the rendering is
ok? Come on.


>> As far as the overwhelming majority of users are concerned, there's
>> no difference between HTML hints and CSS hints.
>
> I expect that the overwhelming majority of users also don't care
> about the difference between alternative stylesheet 1 and alternative
> stylesheet 2. 

They don't know about the feature, maybe, and that is an argument for making it
more visible on pages with alternates. However, users like playing with themes
(just look at mobile phone users changing their login screens). I have yet to
see a user say "I'd like to turn off half the styles on this page".


> So why shouldn't we give the handful who either are
> interested or need it the option of using no stylesheet at all [but leaving
> the HTML hints in place]?

Because every option we add reduces overall usability.


I agree that we should have a feature to remove all the stylesheets, and
according to the CSS spec, HTML hints count as being in a stylesheet.
Comment 82 Paul Arzul 2003-05-06 05:53:10 PDT
> I agree that we should have a feature to remove all the stylesheets,
> and according to the CSS spec, HTML hints count as being in a stylesheet.

do you mean to include user style (userContent.css) when removing all
stylesheets?

i think we need to be careful of reducing accessibility. imagine someone
challenged has gone to the trouble of making their web experience more
accessible, and this feature nukes their style.

as much as i don't like adding options either, i think we need one to keep
user style persistent -- unless we're going to keep it always (which could be
just as bad with a fubar user stylesheet).
Comment 83 Hixie (not reading bugmail) 2003-05-06 05:58:43 PDT
Sorry, I meant author sheets. Yes, of course, user and UA stylesheets would stay
enabled at all times.
Comment 84 Stewart Gordon 2003-05-16 08:03:22 PDT
Re: comment 81

> GD?

Gracefully degrade.

> So you want the user to try various options until they think the
> rendering is ok?  Come on.

They would only need to try one various option in the case I was referring to,
and that is the Use Style -> None that is the subject of this whole bug.  Which
is worse: not being able to read a page at all, or having an option making the
page readable?

> Because every option we add reduces overall usability.

How can a little extra item on the Use Style submenu possibly reduce the
usability of the whole of Mozilla, regardless of whether the user actually uses
the option?
Comment 85 Hixie (not reading bugmail) 2003-05-16 08:59:54 PDT
>> So you want the user to try various options until they think the
>> rendering is ok?  Come on.
>
> They would only need to try one various option in the case I was referring to,
> and that is the Use Style -> None that is the subject of this whole bug.  
> Which is worse: not being able to read a page at all, or having an option 
> making the page readable?

I completely agree with the request for the feature to disable all author
styling. What I was diagreeing with is having the option to disable author
styling done by CSS but _not_ disabling author styling done by HTML.


>> Because every option we add reduces overall usability.
>
> How can a little extra item on the Use Style submenu possibly reduce the
> usability of the whole of Mozilla, regardless of whether the user actually 
> uses the option?

It's a basic fact of user interface design that every additional menu item
increases the time required by the user to decide which menu item to pick.
Comment 86 Jeremy M. Dolan 2003-05-16 09:35:13 PDT
I'd like to mention the two big reasons I see on why this is really a legitimate
feature.

1) It's useful to advanced or disabled users, who have personal stylesheets. Try
setting text or/and background color on the user side and see how many pages are
now unreadable because they only set one or the other.

2) It's useful to developers (at least as much as JS or Java console) to make
sure their page is logicially laid out (section headers should still be <h1>/etc
regardless of how the page looks with style, and things like that).

This is assuming 'None' also removes old HTML styling. <font> and the like.
Otherwise it's not much use, at least for point (1), the more important of the
two IMO.

Also keep in mind this isn't adding a menu item, it's adding a sub-menu item,
which is a big difference.

I for one will be very sad to see the Mozilla-suite featureness attack the
Firebird UI, but the style menu is one addition I look forward to. Heck, doesn't
the spec require it?
Comment 87 Adam Hauner 2003-05-16 12:20:58 PDT
BTW Opera 7.x has this feature: View | Style | User mode
Comment 88 Mats Palmgren (:mats) 2003-08-11 09:33:27 PDT
*** Bug 215803 has been marked as a duplicate of this bug. ***
Comment 89 Hixie (not reading bugmail) 2003-10-26 03:07:38 PST
bz: Will your stylesheet loader work give us an easy way to do this? (Including
disabling author styles in the markup, such as <font> or <hr color>.)
Comment 90 Boris Zbarsky [:bz] 2003-10-26 08:49:20 PST
No, that's pretty orthogonal to this rfe...
Comment 91 Boris Zbarsky [:bz] 2004-02-08 20:34:58 PST
*** Bug 233471 has been marked as a duplicate of this bug. ***
Comment 92 Boris Zbarsky [:bz] 2004-04-20 15:28:32 PDT
*** Bug 241139 has been marked as a duplicate of this bug. ***
Comment 93 Dave Miller [:justdave] (justdave@bugzilla.org) 2004-04-20 17:33:57 PDT
Am I mistaken, or does Firefox already have this feature?  When I visit sites
that have stylesheets, I get a little selector widget in the left end of the
status bar that lets you choose between "No Style", "Basic Style", and the
author's stylesheet.  Is that something that was implemented specifically in
Firefox that didn't get backported to Mozilla?  Or am I looking at something else?
Comment 94 Boris Zbarsky [:bz] 2004-04-20 18:20:04 PDT
> choose between "No Style", "Basic Style", and the author's stylesheet

A vanilla firefox build (just downloaded today's nightly) has a slightly
different widget without the "No Style" option, so sounds like you have some
sort of extension installed (which doesn't do quite what we want to do, since it
can't possibly disable presentational attributes....)
Comment 95 Dave Miller [:justdave] (justdave@bugzilla.org) 2004-04-20 19:27:59 PDT
Created attachment 146651 [details]
Firefox Theme widget

OK, here's what I was seeing.  I mis-remembered it, it says "Theme", not
"Style".  But there is a "No Theme" item, which when chosen does indeed get rid
of any formatting on the page that wasn't defined directly in the HTML.  The
"Basic Theme" item appears to honor fonts, but ignore colors.
Comment 96 Boris Zbarsky [:bz] 2004-04-20 19:30:15 PDT
Yeah, "No Theme" is not there in a vanilla nightly (and again, doesn't do what
we feel it should).
Comment 97 Dave Miller [:justdave] (justdave@bugzilla.org) 2004-04-20 19:47:24 PDT
this is the 0416 nightly, and the only extension I have installed is UserAgent
Switcher.
Comment 98 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-04-20 20:00:23 PDT
It depends on what page is being displayed (and whether it has any persistent
sheets).
Comment 99 fantasai 2004-05-01 00:17:09 PDT
bz, if one were to implement this, what is the interface function the Use Style
> None menu would call?
Comment 100 Boris Zbarsky [:bz] 2004-05-01 00:18:58 PDT
There isn't such a function yet.  It'd have to be written.
Comment 101 Felix Miata 2004-05-01 00:34:00 PDT
We now have a menu option "use style". All we need to do is switch to an empty
stylesheet, no?
Comment 102 Boris Zbarsky [:bz] 2004-05-01 00:39:15 PDT
Felix, please read David Baron's comments on this bug.
Comment 103 fantasai 2004-05-01 00:52:30 PDT
write the interface definition, please?
Comment 104 Boris Zbarsky [:bz] 2004-05-01 01:10:50 PDT
We haven't even quite decided what the function should do.

Once we have an idea of that, we could try to write an interface definition....
(what interface to put it in?  What should happen with subframes?)
Comment 105 fantasai 2004-05-27 15:38:45 PDT
I'm disabling Doc and PresHint. Should this option disable Override as well?
Comment 106 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-05-27 18:07:53 PDT
It probably should disable override in case we ever implement
http://www.w3.org/TR/DOM-Level-2-Style/css.html#CSS-DocumentCSS .  I think
override is currently used only by editor, though.  If you were to add disable
CSS UI to editor that would be a problem, but otherwise I don't think it would.
Comment 107 Karl Heiz Hinterseeer 2004-06-02 07:21:35 PDT
it should be possible to disable any HTML page provided styles and show the page
without any single <STYLE > tag or style="" parameter.
Comment 108 fantasai 2004-07-10 12:38:19 PDT
Created attachment 152786 [details] [diff] [review]
patch
Comment 109 Christian :Biesinger (don't email me, ping me on IRC) 2004-07-10 13:12:27 PDT
nsIDOMDocumentStyle.idl claims it's frozen, as visible in your patch... that
means you should probably not change it.
Comment 110 fantasai 2004-07-11 08:33:58 PDT
I'd have gone through some other interface, but adding that property has been
planned in bug 200930, so I figured I might as well add it now.
Comment 111 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-07-11 10:42:27 PDT
Frozen means you can't change it.

You need to add another interface that inherits from it and add to that instead.
 (This is often done by adding nsIDOMNS... to inherit from nsIDOM...)
Comment 112 Hixie (not reading bugmail) 2004-07-12 02:11:36 PDT
(This also disables things like bgcolor="", <font>, etc, right?)
Comment 113 fantasai 2004-07-12 04:14:32 PDT
(See comment #105. Would I write a patch that didn't?)
Comment 114 fantasai 2004-07-18 14:14:23 PDT
Created attachment 153604 [details] [diff] [review]
patch

Added nsIDOM3DocumentStyle from bz's work on stylesheet switching interfaces
Comment 115 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-07-23 14:36:44 PDT
Comment on attachment 153604 [details] [diff] [review]
patch

>+  //Enable/Disable entire author style level (Doc & PresHint levels)
>+  PRBool GetAuthorStyleDisabled();
>+  nsresult SetAuthorStyleDisabled(PRBool aStyleDisabled);

This could just return void.

>   PRUint16 mBatching;
> 
>   unsigned mInShutdown : 1;
>-  unsigned mDirty : 7;  // one dirty bit is used per sheet type
>+  unsigned mAuthorStyleDisabled;
>+  unsigned mDirty : 6;  // one dirty bit is used per sheet type

The default is 32 bits, so putting that in the middle there doesn't do what you
intended, I don't think.  That said, please don't change this to 6, even though
it could be now, since I'm adding a 7th in bug 252578.

You'll also have to merge the GatherRuleProcessors changes with bug 252578 if I
land first -- and it would probably be simpler to use an early return i.e.,
after the Clear(), just return if mAuthorStyleDisabled and the type is one of
the relevant types.  (I'm saying that knowing what the changes in bug 252578
look like.)

More later...
Comment 116 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-07-23 14:38:56 PDT
(In reply to comment #115)
> (From update of attachment 153604 [details] [diff] [review])
> >+  //Enable/Disable entire author style level (Doc & PresHint levels)
> >+  PRBool GetAuthorStyleDisabled();
> >+  nsresult SetAuthorStyleDisabled(PRBool aStyleDisabled);
> 
> This could just return void.

Actually, never mind.
Comment 117 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-07-27 17:58:57 PDT
Comment on attachment 153604 [details] [diff] [review]
patch

>+nsStyleSet::SetAuthorStyleDisabled(PRBool aStyleDisabled)
>+{
>+  if (aStyleDisabled != (PRBool)mAuthorStyleDisabled) {

Don't cast to PRBool; it doesn't help anything.

>+/**
>+ * The nsIDOM3DocumentStyle interface is an extension to the
>+ * nsIDOMDocumentStyle interface.  This interface exposes more ways to interact

If it's an extension, shouldn't it be nsIDOMNSDocumentStyle?

What happened to all the nsDocument changes in bz's patch in bug 200930?  The
preferredStyleSheet changes and the authorStyleDisabled stuff seem like two
different things that should be reviewed separately.  And the UI changes should
probably be reviewed as a third patch (or third and fourth, if separate for the
suite and Firefox).

Any chance you could attach a new patch addressing these (and previous)
comments to this bug, and a separate one to bug 200930 if that one needs
updating?  Other than that, the authorStyleDisabled part of this patch looks
fine.
Comment 118 fantasai 2004-07-31 04:28:18 PDT
> If it's an extension, shouldn't it be nsIDOMNSDocumentStyle?

I haven't got a clue. I don't know anything about APIs, I'm just copying what bz
did in bug 200930. If there's something you want me to change about it, you need
to explain it.

> What happened to all the nsDocument changes in bz's patch in bug 200930?

Most of those changes are for adding document.selectedStylesheetSet support and
are therefore not relevant here.

> The preferredStyleSheet changes and the authorStyleDisabled stuff seem like
> two different things that should be reviewed separately.

Although it adds a new interface, the preferredStylesheet code changes are small
and they are necessary for a correct interface here. I don't see how separating
out the patch will help, it's just more work afaics.

As for not using bz's method of calling GetHeaderData to get the preferred style
set, I don't think it would work. The preferred style sheet is not always set
via HTTP headers.
See http://www.w3.org/TR/html40/present/styles.html#h-14.3.2

If you want to spend more time pondering the preferredStylesheetSet interface,
then I will file a new bug on it. But if you already know what the interface is
going to be, I don't see any point.

> And the UI changes should probably be reviewed as a third patch (or third and
> fourth, if separate for the suite and Firefox).

I will open a separate bug for the patch to Firefox trunk. The UI changes for
the Mozilla Suite are necessary in this patch because there's no other way to
test the changes! Besides, if the only way of doing that is for the user to make
an API call, then /this/ bug is definitely not fixed and will need a second
patch.
Comment 119 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-07-31 13:35:02 PDT
One other thing -- the JS changes make it look like you're removing the "Basic
Page Style" option whenever there are preferred sets.  This make sense, but if
so, perhaps the name should be "Page Style" instead?  Also, if you think the
code would be simpler if that option were created by the function that fills the
popup (and created only if there are no preferred/alternate sheets), then that
might be better (and more symmetric, since we're already doing that, and having
both that and display toggling might be confusing).
Comment 120 neil@parkwaycc.co.uk 2004-07-31 14:15:04 PDT
Comment on attachment 153604 [details] [diff] [review]
patch

>-          <menupopup onpopupshowing="stylesheetFillPopup(this);"
>-                     oncommand="stylesheetSwitchAll(window.content, event.target.getAttribute('data'));">
>-            <menuitem label="&useStyleSheetPersistentOnly.label;" accesskey="&useStyleSheetPersistentOnly.accesskey;" type="radio"/>
>+            <menuitem label="&useStyleSheetNone.label;" accesskey="&useStyleSheetNone.accesskey;" oncommand="setStyleDisabled(true);" type="radio"/>
I don't like you putting oncommand attribute on your dynamic menuitems. Please
add event.preventBubble(); here instead.
>+            <menuitem label="&useStyleSheetPersistentOnly.label;" accesskey="&useStyleSheetPersistentOnly.accesskey;"
>+                      oncommand="stylesheetSwitchAll(window.content, event.target.getAttribute('data'));" type="radio"/>

>-  itemNoOptStyles.setAttribute("checked", noOptionalStyles);
>+  menuPopup.firstChild.setAttribute("checked", styleDisabled);
>+  itemPersistentOnly.setAttribute("checked", !altStyleSelected && !styleDisabled);
Why can't you keep using the noOptionalStyles variable? Also you didn't remove
all references to it.

>+  if (window.content.document.preferredStylesheetSet) {
>+    itemPersistentOnly.style.display = "none";
>+  }
>+  else {
>+    itemPersistentOnly.style.display = "";
>+  }
We have to use .hidden = <boolean> here to play nicely with the Mac whose
menubar doesn't use CSS, although in general we like to use attributes and
stylesheets (.hidden = true; invokes a style rule in xul.css) rather than
inline style.

> function stylesheetSwitchAll(frameset, title) {
>+  setStyleDisabled(false);
This is a recursive function so this isn't a good place to add code. As you'll
be moving the oncommand handler back to the menupopup you could add it there.
Additionally I would have thought that enabling style after instead of before
selecting the stylesheet would reduce the amount of style resolving.
>   if (!title || stylesheetInFrame(frameset, title)) {
>     stylesheetSwitchFrame(frameset, title);
>   }
>   for (var i = 0; i < frameset.frames.length; i++) {
>     stylesheetSwitchAll(frameset.frames[i], title);
>   }
> }
Comment 121 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-07-31 14:17:48 PDT
(In reply to comment #120)
> >-  itemNoOptStyles.setAttribute("checked", noOptionalStyles);
> >+  menuPopup.firstChild.setAttribute("checked", styleDisabled);
> >+  itemPersistentOnly.setAttribute("checked", !altStyleSelected &&
!styleDisabled);
> Why can't you keep using the noOptionalStyles variable? Also you didn't remove
> all references to it.

It's just a rename of the same thing to be more consistent with other parts of
the code.
Comment 122 fantasai 2004-08-01 08:07:49 PDT
Created attachment 154923 [details] [diff] [review]
patch
Comment 123 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-08-01 11:29:02 PDT
Comment on attachment 154923 [details] [diff] [review]
patch

>Index: mozilla/content/base/src/nsStyleSet.h
>   unsigned mInShutdown : 1;
>-  unsigned mDirty : 7;  // one dirty bit is used per sheet type
>+  unsigned mAuthorStyleDisabled: 1;
>+  unsigned mDirty : 6;  // one dirty bit is used per sheet type

We need 7 bits here, so you can't use 6.

>Index: mozilla/content/base/src/nsStyleSet.cpp
> nsresult
> nsStyleSet::GatherRuleProcessors(sheetType aType)
> {
>   mRuleProcessors[aType] = nsnull;
>+  if (mAuthorStyleDisabled && (aType == eDocSheet || aType == ePresHintSheet)) {
>+    //don't regather if this level is disabled
>+    return NS_OK;
>+  }

You need to check eHTMLPresHintSheet and eStyleAttrSheet as well.

>+nsresult
>+nsStyleSet::SetAuthorStyleDisabled(PRBool aStyleDisabled)
>+{
>+  if (aStyleDisabled != mAuthorStyleDisabled) {
>+    mAuthorStyleDisabled = aStyleDisabled;
>+    if (!mBatching) {
>+      nsresult rv;
>+      rv = GatherRuleProcessors(eDocSheet);
>+      if (NS_FAILED(rv)) return rv;
>+      rv = GatherRuleProcessors(ePresHintSheet);
>+      if (NS_FAILED(rv)) return rv;
>+    }
>+    else {
>+      mDirty |= 1 << eDocSheet;
>+      mDirty |= 1 << ePresHintSheet;
>+    }
>+  }
>+  return NS_OK;
>+}

Likewise here.	Perhaps it's worth having an array:

static const nsStyleSet::sheetType authorTypes = {
  ePresHintSheet,
  eHTMLPresHintSheet,
  eDocSheet,
  eStyleAttrSheet
};

and iterating through that array in both these places, e.g. (for the first):

// don't regather if author style is disabled
if (mAuthorStyleDisabled)
  for (const nsStyleSet::sheetType *type = authorTypes,
			       *type_end = authorTypes +
NS_ARRAY_LENGTH(authorTypes);
       type < type_end; ++type)
    if (*type == aType)
      return NS_OK;

although perhaps it's not worth it for just 4 entries.	(I wonder which one
generates less code.)

>Index: mozilla/dom/public/idl/stylesheets/nsIDOMNSDocumentStyle.idl
>+ * The nsIDOM3DocumentStyle interface is an extension to the

The comment should match the name.

With those changes, sr=dbaron.
Comment 124 fantasai 2004-08-01 15:39:15 PDT
Created attachment 154939 [details] [diff] [review]
patch
Comment 125 fantasai 2004-08-01 16:16:45 PDT
Created attachment 154942 [details] [diff] [review]
patch
Comment 126 neil@parkwaycc.co.uk 2004-08-02 02:54:07 PDT
Comment on attachment 154942 [details] [diff] [review]
patch

I was surprised that turning all style off also affected mapped attributes.
Anyway, I was originally unsure how this was supposed to work, but now I see
that "None" means no CSS at all while "Default Style" means that there were no
named regular stylesheets.
Comment 127 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-08-03 17:17:08 PDT
Comment on attachment 154942 [details] [diff] [review]
patch

>Index: mozilla/content/base/src/nsStyleSet.h
>-  unsigned mDirty : 7;  // one dirty bit is used per sheet type
>+  unsigned mAuthorStyleDisabled: 1;
>+  unsigned mDirty : 14;  // one dirty bit is used per sheet type

Just leave it at 7, since that shows what we need.


>Index: mozilla/content/base/src/nsStyleSet.cpp
>+nsresult
>+nsStyleSet::SetAuthorStyleDisabled(PRBool aStyleDisabled)
>+{
>+  if (aStyleDisabled != mAuthorStyleDisabled) {
>+    mAuthorStyleDisabled = aStyleDisabled;
>+    if (!mBatching) {
>+      nsresult rv;
>+      rv = GatherRuleProcessors(eDocSheet);
>+      if (NS_FAILED(rv)) return rv;
>+      rv = GatherRuleProcessors(ePresHintSheet);
>+      if (NS_FAILED(rv)) return rv;
>+      rv = GatherRuleProcessors(eHTMLPresHintSheet);
>+      if (NS_FAILED(rv)) return rv;
>+      rv = GatherRuleProcessors(eHTMLPresHintSheet);
>+      if (NS_FAILED(rv)) return rv;
>+    }
>+    else {
>+      mDirty |= 1 << eDocSheet;
>+      mDirty |= 1 << ePresHintSheet;
>+      mDirty |= 1 << eHTMLPresHintSheet;
>+      mDirty |= 1 << eStyleAttrSheet;
>+    }
>+  }
>+  return NS_OK;

I just realized a way to simplify this a good bit:

  if (aStyleDisabled != mAuthorStyleDisabled) {
    mAuthorStyleDisabled = aStyleDisabled;
    BeginUpdate();
    mDirty |= 1 << eDocSheet |
	      1 << ePresHintSheet |
	      1 << eHTMLPresHintSheet |
	      1 << eStyleAttrSheet;
    EndUpdate();
  }
  return NS_OK;

With that, sr=dbaron.
Comment 128 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-08-03 17:28:11 PDT
... except that the return value from EndUpdate should be propagated, i.e.,
|nsresult rv = NS_OK;| at the start of the function, |rv = EndUpdate();| instead
of just |EndUpdate();|, and |return rv;| instead of |return NS_OK;|.
Comment 129 fantasai 2004-08-04 14:14:23 PDT
Created attachment 155203 [details] [diff] [review]
final patch
Comment 130 Peter Van der Beken [:peterv] 2004-08-05 05:31:34 PDT
(In reply to comment #118)
> As for not using bz's method of calling GetHeaderData to get the preferred style
> set, I don't think it would work. The preferred style sheet is not always set
> via HTTP headers.

But the headerDefaultStyle header data is always set to the preferred style
sheet. If it didn't work, you need to file a bug. There was no need for
GetPreferredSheet on the CSS loader IMHO.

The license on nsIDOMNSDocumentStyle.idl references the NPL, please use the
MPL/GPL/LGPL boilerplate
(http://www.mozilla.org/MPL/boilerplate-1.1/mpl-tri-license-c) when adding new
files.
Comment 131 Hiro 2004-08-05 05:52:30 PDT
The error came out by build of Camino.
Mac OS X 10.3.4

make[7]: Entering directory
`/Users/sek/Documents/mozilla-current/camino/mozilla/dom/public/idl/stylesheets'
/Users/sek/Documents/mozilla-current/camino/mozilla/config/nsinstall -L
/Users/sek/Documents/mozilla-current/camino/mozilla/dom/public/idl/stylesheets
-m 644 nsIDOMLinkStyle.idl ../../../../dist/idl
/Users/sek/Documents/mozilla-current/camino/mozilla/config/nsinstall -L
/Users/sek/Documents/mozilla-current/camino/mozilla/dom/public/idl/stylesheets
-m 644 _xpidlgen/nsIDOMLinkStyle.h ../../../../dist/include/dom
/usr/bin/perl -I../../../../config ../../../../config/build-list.pl
../../../../dist/include/dom/.headerlist nsIDOMLinkStyle.h
make[7]: *** No rule to make target `nsIDOMNSDocumentStyle.idl', needed by
`export'.  Stop.
make[7]: Leaving directory
`/Users/sek/Documents/mozilla-current/camino/mozilla/dom/public/idl/stylesheets'
make[6]: *** [export] Error 2
Comment 132 fantasai 2004-08-05 05:57:39 PDT
If the defaultStyle header *is* set by the configuration of <link> elements,
/then/ I'd probably have to file a bug. <link> elements shouldn't set HTTP
headers. And if they are, then we're probably violating HTML 4.

http://www.w3.org/TR/html40/present/styles.html#h-14.3.2
  # If two or more META declarations or HTTP headers specify the preferred
  # style sheet, the last one takes precedence. HTTP headers are considered
  # to occur earlier than the document HEAD for this purpose.
  #
  # If two or more LINK elements specify a preferred style sheet, the first
  # one takes precedence.
  #
  # Preferred style sheets specified with META or HTTP headers have precedence
  # over those specified with the LINK element."

How would we implement that if a default style set with a LINK element gets
treated the same as a meta tag with an HTTP header?
Comment 133 Peter Van der Beken [:peterv] 2004-08-05 06:54:55 PDT
They are not treated the same, but they all set the headerDefaultStyle header
data. There could very well be bugs there, that still doesn't justify
GetPreferredSheet on the CSS loader. If our existing API really is broken you
did nothing to fix it, you just added another API to get the same bad value.
Comment 134 fantasai 2004-08-05 08:49:29 PDT
-> bug 254445
marking this fixed. Tinderbox is in flames because cvs-mirror is out of sync 
with cvs.
Comment 135 Darin Fisher 2004-08-27 18:39:39 PDT
It looks like nsIMarkupDocumentViewer was changed without modifying the UUID of
the interface.  Sure, it's a private interface, but you never know if some wacky
extension or embedder may be using this interface for some random reason. 
Better to always modify interface UUIDs when modifying interfaces.  Otherwise,
you give those who need to use this interface no hope of doing so across browser
versions.
Comment 136 Christian :Biesinger (don't email me, ping me on IRC) 2004-08-30 12:02:54 PDT
(In reply to comment #135)
> It looks like nsIMarkupDocumentViewer was changed without modifying the UUID of
> the interface.

I just checked in a patch to change the uuid. unfortunately alpha3 was released
with the old UUID, but ah well, it's only an alpha.
Comment 137 Jo Hermans 2004-10-18 02:19:28 PDT
*** Bug 264859 has been marked as a duplicate of this bug. ***
Comment 138 Karl Heiz Hinterseeer 2005-01-31 20:49:12 PST
while netscape4.8 is able to show html pages when offline (lacking the css which
points to a net adress) if you disable the css use, Mozilla 2005013106 is not,
regardless if you choose View-Use Style-None or not.
Comment 139 Boris Zbarsky [:bz] 2006-04-04 17:23:35 PDT
So why was this _experimental_ interface added to SDK_XPIDLSRCS?  And given that I need to change it if I'm going to reasonably implement the WHATWG proposal for this stuff, where do we go from here?

I'm tempted to move it out of SDK_XPIDLSRCS, frankly.  Except I suspect that's not kosher.  :(
Comment 140 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2006-04-04 17:28:54 PDT
Yikes.  I suppose if you have to, you can just leave it and not implement it anymore.  Although not being able to use the name anymore is annoying.
Comment 141 Boris Zbarsky [:bz] 2006-04-04 23:13:54 PDT
So the current WHATWG proposal would just extend this interface.  I suppose I could create nsIDOMNSDocumentStyle2 and inherit from nsIDOMNSDocumentStyle...
Comment 142 fantasai 2006-04-05 09:33:53 PDT
I'd call it nsIDOMNSDocumentStyleSets over nsIDOMNSDocumentStyle2
Comment 143 Daniel Glazman (:glazou) 2006-04-05 09:59:23 PDT
(In reply to comment #142)
> I'd call it nsIDOMNSDocumentStyleSets over nsIDOMNSDocumentStyle2

Agreed ! Please, please don't run into the MS way of numbering new interfaces...
Comment 144 Boris Zbarsky [:bz] 2006-04-05 11:45:32 PDT
> I'd call it nsIDOMNSDocumentStyleSets over nsIDOMNSDocumentStyle2

How would someone looking at the WHATWG spec (where the interface is DocumentStyle) find that?  I don't like the numbering that much, but it's the only sane choice when the DOM or WHATWG folks add stuff to interfaces that are already frozen.

Note the discussion in mozilla.dev.platform, btw.  I'm still tempted to just move this interface back out of SDK_XPIDLSRCS.
Comment 145 Darin Fisher 2006-04-05 12:49:21 PDT
The interface name only affects C++ consumers at least.  We've been using numbering for other DOM interfaces (e.g., nsIDOM3Node anyone?).
Comment 146 Darin Fisher 2006-04-05 12:50:31 PDT
nsIDOMWindow2 is probably a better example of my point than nsIDOM3Node fwiw.

Note You need to log in before you can comment on or make changes to this bug.