Last Comment Bug 513461 - Need pseudo classes for lightweight theme handling
: Need pseudo classes for lightweight theme handling
Status: RESOLVED FIXED
: dev-doc-complete, perf
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: P1 normal (vote)
: mozilla1.9.3a1
Assigned To: Nochum Sossonko [:Natch]
:
Mentors:
Depends on: 528792 547608
Blocks: 511107
  Show dependency treegraph
 
Reported: 2009-08-29 07:51 PDT by Dão Gottwald [:dao]
Modified: 2010-09-30 10:53 PDT (History)
8 users (show)
roc: blocking1.9.2+
highmind63: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta1-fixed


Attachments
wip 1 (15.52 KB, patch)
2009-09-03 21:43 PDT, Nochum Sossonko [:Natch]
no flags Details | Diff | Review
patch (58.20 KB, patch)
2009-09-06 15:30 PDT, Nochum Sossonko [:Natch]
no flags Details | Diff | Review
patch (28.96 KB, patch)
2009-09-06 15:40 PDT, Nochum Sossonko [:Natch]
no flags Details | Diff | Review
patch ver. 1.1 (38.30 KB, patch)
2009-09-08 11:35 PDT, Nochum Sossonko [:Natch]
dao+bmo: review-
Details | Diff | Review
patch ver. 1.2 (36.27 KB, patch)
2009-09-08 17:23 PDT, Nochum Sossonko [:Natch]
dao+bmo: review-
Details | Diff | Review
patch ver. 1.3 (36.40 KB, patch)
2009-09-14 18:33 PDT, Nochum Sossonko [:Natch]
no flags Details | Diff | Review
patch ver. 1.4 (36.79 KB, patch)
2009-09-16 16:09 PDT, Nochum Sossonko [:Natch]
dao+bmo: review+
Details | Diff | Review
patch ver. 1.5 (7.36 KB, patch)
2009-09-22 16:37 PDT, Nochum Sossonko [:Natch]
no flags Details | Diff | Review
patch ver. 1.5 (36.59 KB, patch)
2009-09-22 16:38 PDT, Nochum Sossonko [:Natch]
dbaron: review+
Details | Diff | Review
patch ver. 1.6 (36.13 KB, patch)
2009-10-01 20:29 PDT, Nochum Sossonko [:Natch]
no flags Details | Diff | Review
follow-up: reorder initializers in constructor, to fix build warning (961 bytes, patch)
2009-10-13 16:11 PDT, Daniel Holbert [:dholbert] (largely AFK until June 28)
dbaron: review+
Details | Diff | Review

Description Dão Gottwald [:dao] 2009-08-29 07:51:29 PDT
I'm currently using many selectors like ':root[lwtheme="true"] toolbar' in bug 511107, but we'd like to avoid descendant selectors. To achieve that, I'm proposing the following pseudo classes:

:-moz-lwtheme
Matches in chrome documents when the root element's lwtheme attribute is "true".

:-moz-lwtheme-darktext
Matches when :-moz-lwtheme matches and the root element's lwthemetextcolor attribute is "dark".

:-moz-lwtheme-brighttext
Matches when :-moz-lwtheme matches and the root element's lwthemetextcolor attribute is "bright".
Comment 1 Nochum Sossonko [:Natch] 2009-09-03 21:43:23 PDT
Created attachment 398578 [details] [diff] [review]
wip 1

Based on the patch in bug 478416 from Neil Deakin, this does the same thing for :-moz-lwtheme and :-moz-lwtheme-is(bright|dark).

This needs some tests. Also, in nsCSSRuleProcessor I'm not sure of a better way to assign the result aside for using 1 and 2, should I make a local enum that duplicates what nsXULDocument has?

This also doesn't work well with getComputedStyle, the style returned is the style of the pseudo selector, without specifying it. Specifying the pseudo selector does absolutely nothing afaict.
Comment 2 Nochum Sossonko [:Natch] 2009-09-03 21:53:21 PDT
Oh, and currently -moz-lwtheme only matches if 1) lwtheme="true" and 2) there's a valid value in lwthemetextcolor (either dark or bright) on the root element. I figured that's the intention, correct me if I'm wrong.
Comment 3 Markus Stange [:mstange] [away until June 27] 2009-09-03 22:07:48 PDT
Just FYI, this will probably conflict with my patch in bug 508482.
Comment 4 Dão Gottwald [:dao] 2009-09-04 05:50:07 PDT
Nochum, would you also like to update the stylesheets that use the attributes or should I prepare that patch?
Comment 5 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-09-04 06:08:41 PDT
Comment on attachment 398578 [details] [diff] [review]
wip 1

It would be good if you added a basic parsability test to layout/style/test/test_selectors.html (which will also test serialization and cloning a little bit) in addition to other tests you need.

>+   * Returns 0 if the document lightweight theme is uninitialized, 1 for no
>+   * theme, 2 for a dark theme and 3 for a light theme. This is used to
>+   * determine the state of the pseudoclasses :-moz-lwtheme and :-moz-lwtheme-is.

This sounds like what we use enums for.  I think you should move the enum you declare in nsXULDocument to nsIDocument so that this comment can use names rather than numbers.

>+        // if the lwtheme changed, make sure to restyle appropriately
>+        if ((aName == nsGkAtoms::lwtheme ||
>+             aName == nsGkAtoms::lwthemetextcolor) &&
>+            document && document->GetRootContent() == this) {
>+            nsCOMPtr<nsIXULDocument> xuldoc = do_QueryInterface(document);
>+            if (xuldoc) {
>+                xuldoc->ResetDocumentLWTheme();
>+            }
>+        }

It's possible we might want to use the document state mechanism that mstange is adding in bug 508482.  I think it has less direct benefit here, but there is value in using similar code.  I guess it's probably ok as it is now, though, since it's similar to the localedir code.

However, the comment here is wrong:  this code isn't doing any restyling:  it's just clearing the XUL document's cache.  The restyling is being done in nsCSSRuleProcessor::HasAttributeDependentStyle.

>diff --git a/layout/style/nsCSSParser.cpp b/layout/style/nsCSSParser.cpp
>--- a/layout/style/nsCSSParser.cpp
>+++ b/layout/style/nsCSSParser.cpp
>@@ -3323,16 +3323,24 @@ CSSParserImpl::ParsePseudoClassWithIdent
>   // -moz-locale-dir can only have values of 'ltr' or 'rtl'.
>   if (aPseudo == nsCSSPseudoClasses::mozLocaleDir) {
>     if (!mToken.mIdent.EqualsLiteral("ltr") &&
>         !mToken.mIdent.EqualsLiteral("rtl")) {
>       return eSelectorParsingStatus_Error;
>     }
>   }
> 
>+  // -moz-lwtheme-is can only have values of 'dark' or 'bright'.
>+  if (aPseudo == nsCSSPseudoClasses::mozLWThemeIs) {
>+    if (!mToken.mIdent.EqualsLiteral("dark") &&
>+        !mToken.mIdent.EqualsLiteral("bright")) {
>+      return eSelectorParsingStatus_Error;

Hmmm.  These should be case-insensitive, and so should the :-moz-locale-dir() arguments.  And it looks like they're not, although maybe I'm missing something.

>+// -moz-lwtheme may be used to match a document that has a lightweight theme
>+CSS_PSEUDO_CLASS(mozLWTheme, ":-moz-lwtheme")
>+
>+// -moz-lwtheme-is(dark) and -moz-lwtheme-is(bright) may be used to match the
>+// lightweight theme being used on the document
>+CSS_PSEUDO_CLASS(mozLWThemeIs, ":-moz-lwtheme-is")

I wonder if it would be better to call these :-moz-has-lwtheme and :-moz-lwtheme(dark)
Comment 6 Dão Gottwald [:dao] 2009-09-04 06:22:22 PDT
-moz-lwtheme-is(dark) is wrong. This is about the text color, hence the proposed :-moz-lwtheme-darktext and :-moz-lwtheme-brighttext.
Comment 7 Nochum Sossonko [:Natch] 2009-09-04 15:04:19 PDT
I'll update this on Sunday (hopefully), as to the names, how about -moz-has-lwtheme and -moz-lwtheme-text(dark|bright)? I'm trying to avoid adding more pseudo-selectors if possible. Also, this is more extensible.

I'll update the stylesheets and add a test as well.

About the casing, I can make it case-insensitive, I was just following what -moz-locale-dir was doing, do you want me to update that too?
Comment 8 Dão Gottwald [:dao] 2009-09-04 15:11:38 PDT
(In reply to comment #7)
> I'm trying to avoid adding more pseudo-selectors if possible.

To what end?
I'd like to have this easily typeable. I also find :-moz-locale-dir(rtl) instead of :-moz-rtl-locale unfortunate.

> Also, this is more extensible.

I don't foresee us adding more distinctions, other than maybe neither dark nor bright, which would be no change from the style system perspective.
Comment 9 Nochum Sossonko [:Natch] 2009-09-06 15:30:26 PDT
Created attachment 398980 [details] [diff] [review]
patch

Patch ready for review, including the parsing test and changed the names in accordance with the comments. The stylesheets have been updated as well.
Comment 10 Nochum Sossonko [:Natch] 2009-09-06 15:31:02 PDT
Comment on attachment 398980 [details] [diff] [review]
patch

Dao, can you review the stylesheet changes?
Comment 11 Nochum Sossonko [:Natch] 2009-09-06 15:40:58 PDT
Created attachment 398982 [details] [diff] [review]
patch

Sorry for the bugspam, patch was full of cruft from earlier attempts and some line-endings were changed unintentionally.
Comment 12 Dão Gottwald [:dao] 2009-09-06 15:50:08 PDT
Why is :-moz-has-lwtheme better than :-moz-lwtheme?
Comment 13 Dão Gottwald [:dao] 2009-09-06 16:06:20 PDT
Comment on attachment 398982 [details] [diff] [review]
patch

>+#main-window:-moz-has-lwtheme:not([active="true"]) .tabbrowser-tab {

In preparation for bug 508482, this should be:
#main-window:not([active="true"]) .tabbrowser-tab:-moz-has-lwtheme

Looks good otherwise. I'm going to test all three themes, as I think some selectors now have less weighting, and maybe not enough in some cases.
Comment 14 Nochum Sossonko [:Natch] 2009-09-06 18:12:24 PDT
I implemented :-moz-has-lwtheme based on dbaron's comment. It seems more self-explanatory that way too. The selectors may indeed have gone a bit awry, I noticed that bookmarks in the bookmark toolbar get a very thick bold text when the theme is set to bright, yet when you hover over them they return to their regular font weight. I wasn't sure if that was the intention so I figured I'd mention it here just in case it's a side-effect of losing specificity and therefore applicability.
Comment 15 Dão Gottwald [:dao] 2009-09-07 01:48:55 PDT
(In reply to comment #14)
> I implemented :-moz-has-lwtheme based on dbaron's comment. It seems more
> self-explanatory that way too.

The element doesn't have the lightweight theme, though -- the document has. (Like so many other things, maybe this should be an @ rule.)
Comment 16 Dão Gottwald [:dao] 2009-09-07 02:03:48 PDT
Comment on attachment 398982 [details] [diff] [review]
patch

>--- a/toolkit/themes/gnomestripe/global/global.css
>+++ b/toolkit/themes/gnomestripe/global/global.css

>+toolbar:-moz-has-lwtheme,

>+menubar:-moz-has-lwtheme > menu,
>+toolbarbutton:-moz-has-lwtheme {

>+menubar:-moz-has-lwtheme > menu:not([open="true"]),
>+toolbarbutton:-moz-has-lwtheme:not(:hover):not([checked="true"]):not([open="true"]):not([disabled="true"])

>--- a/toolkit/themes/pinstripe/global/global.css
>+++ b/toolkit/themes/pinstripe/global/global.css

>+toolbar:-moz-has-lwtheme,
>+findbar:-moz-has-lwtheme,

>+splitter:-moz-has-lwtheme {

>+.findbar-container:-moz-has-lwtheme > label,
>+.findbar-find-status:-moz-has-lwtheme {

>--- a/toolkit/themes/winstripe/global/global.css
>+++ b/toolkit/themes/winstripe/global/global.css

>+toolbox:moz-has-lwtheme,
>+toolbar:moz-has-lwtheme,

>+splitter:moz-has-lwtheme {

>+toolbarbutton:moz-has-lwtheme {

>+menubar:moz-has-lwtheme > menu {

>+toolbarbutton:moz-has-lwtheme:not(:hover):not([checked="true"]):not([open="true"]):not([disabled="true"]) {

now that they don't depend on the root element anymore, these should probably be moved to toolbar.css / menu.css / toolbarbutton.css / findBar.css / splitter.css.

>--- a/toolkit/themes/winstripe/global/global.css
>+++ b/toolkit/themes/winstripe/global/global.css

>+.tabbrowser-tabs:moz-has-lwtheme {

This actually belongs in winstripe's browser.css. (This was wrong in the original patch.)
Comment 17 Nochum Sossonko [:Natch] 2009-09-08 11:35:18 PDT
Created attachment 399264 [details] [diff] [review]
patch ver. 1.1

This patch addresses all of Dao's previous comments.
Comment 18 Dão Gottwald [:dao] 2009-09-08 11:54:03 PDT
Comment on attachment 399264 [details] [diff] [review]
patch ver. 1.1

I still think -moz-has-lwtheme is wrong. While it is more self-explanatory, it's also misleading. It makes sense for the root element the element with lwthemefooter="true", which have the lightweight theme's background images and text color, but not for the descendant elements.

>+.toolbar-focustarget {
>+  -moz-user-focus: ignore !important;
>+}

This doesn't belong in toolbar.css. (It doesn't belong in any of these stylesheets, but that's another bug.)
Comment 19 Dão Gottwald [:dao] 2009-09-08 14:38:11 PDT
Comment on attachment 399264 [details] [diff] [review]
patch ver. 1.1

>-toolbar[mode="text"] .toolbarbutton-text {
>-  padding: 0 !important;
>-  margin: 3px 5px !important;
>-}

This shouldn't be moved either.

>--- a/toolkit/themes/winstripe/global/menu.css
>+++ b/toolkit/themes/winstripe/global/menu.css

>+menubar:-moz-has-lwtheme > menu {
>+  -moz-appearance: none;
>+  color: inherit;
>+}
> menubar > menu[_moz-menuactive="true"][open="true"] {
>   border-width: 3px 1px 1px 3px;
> }
> menubar > menu[_moz-menuactive="true"],
> menubar > menu[_moz-menuactive="true"][open="true"] {
>   color: -moz-menubarhovertext;
> }

menubar > menu[_moz-menuactive="true"] seems to override menubar:-moz-has-lwtheme > menu. Adding :not(:-moz-lwtheme) to the former would be one way to solve this.

>+menubar:-moz-has-lwtheme > menu:not([open="true"]) {
>+  color: inherit;
>+  text-shadow: inherit;
>+}

You seem to have moved this from gnomestripe to winstripe.
Comment 20 Nochum Sossonko [:Natch] 2009-09-08 15:26:34 PDT
Dao, can you explain to me what [_moz-menuactive=true] matches so that I can test this before uploading another patch? I think I've worked it out, just need to know what it's for.
Comment 21 Dão Gottwald [:dao] 2009-09-08 15:34:02 PDT
[_moz-menuactive="true"] can be triggered by moving the mouse over the menu, for instance, but it's actually menubar > menu[_moz-menuactive="true"][open="true"] what overrides the other selector, so just open a menu.
Comment 22 Nochum Sossonko [:Natch] 2009-09-08 17:23:52 PDT
Created attachment 399362 [details] [diff] [review]
patch ver. 1.2
Comment 23 Dão Gottwald [:dao] 2009-09-09 14:41:02 PDT
Comment on attachment 399362 [details] [diff] [review]
patch ver. 1.2

>--- a/toolkit/themes/gnomestripe/global/toolbar.css
>+++ b/toolkit/themes/gnomestripe/global/toolbar.css

>+toolbar:-moz-has-lwtheme {
>+  -moz-appearance: none;
>+}
>+
> menubar, toolbar[type="menubar"] {
>   -moz-appearance: menubar;

toolbar[type="menubar"] overrides toolbar:-moz-has-lwtheme here.

I've also noticed a small glitch with checkboxes in gnomestripe, which is not caused by your changes though. We're changing the background and text color on hover, but still inherit the text shadow. Could be fixed by adding text-shadow:none in checkbox.css:

114 checkbox:not([disabled="true"]):hover {
115   color: -moz-buttonhovertext;
116 }

The same applies to radio.css. Both could be moved to a separate bug if you don't want to touch it.


Other than that it seems more self-explanatory, can you explain why you think has-lwtheme is actually correct?
Comment 24 Dão Gottwald [:dao] 2009-09-09 15:07:29 PDT
Comment on attachment 399362 [details] [diff] [review]
patch ver. 1.2

>--- a/toolkit/themes/winstripe/global/toolbar.css
>+++ b/toolkit/themes/winstripe/global/toolbar.css

>+toolbox:-moz-has-lwtheme,
>+toolbar:-moz-has-lwtheme {
>+  -moz-appearance: none;
>+  background: none;
>+  border-style: none;
>+}

toolbar:first-child overrides this, adding back a bottom border.
Comment 25 Dão Gottwald [:dao] 2009-09-09 15:11:52 PDT
Comment on attachment 399362 [details] [diff] [review]
patch ver. 1.2

--- a/toolkit/themes/winstripe/global/global.css
+++ b/toolkit/themes/winstripe/global/global.css

-:root[lwtheme="true"] menubar > menu {
-  -moz-appearance: none;
-  color: inherit;
-}

You haven't moved this to menu.css.
Comment 26 Nochum Sossonko [:Natch] 2009-09-14 18:33:09 PDT
Created attachment 400643 [details] [diff] [review]
patch ver. 1.3

(In reply to comment #25)

Um, I did move it, at least the line you point to in that comment. winstripe/global/menu.css there's on edit, this.

Everything else was addressed.
Comment 27 Dão Gottwald [:dao] 2009-09-15 00:13:54 PDT
(In reply to comment #26)
> Everything else was addressed.

Except for the checkbox changes?
Comment 28 Dão Gottwald [:dao] 2009-09-15 00:34:34 PDT
Comment on attachment 400643 [details] [diff] [review]
patch ver. 1.3

>--- a/toolkit/themes/winstripe/global/menu.css
>+++ b/toolkit/themes/winstripe/global/menu.css

> menubar > menu[_moz-menuactive="true"],
> menubar > menu[_moz-menuactive="true"][open="true"] {
>   color: -moz-menubarhovertext;
> }
>+menubar > menu:-moz-has-lwtheme {
>+  -moz-appearance: none;
>+  color: inherit;
>+}

menubar > menu[_moz-menuactive="true"][open="true"] still overrides this. Probably best to use !important for the color.
Comment 29 Dão Gottwald [:dao] 2009-09-15 00:44:57 PDT
Comment on attachment 400643 [details] [diff] [review]
patch ver. 1.3

>--- a/browser/themes/pinstripe/browser/browser.css
>+++ b/browser/themes/pinstripe/browser/browser.css

>-#main-window[lwtheme="true"] .tabbrowser-tab,
>-#main-window[lwtheme="true"]:not([active="true"]) .tabbrowser-tab {
>+.tabbrowser-tab:-moz-has-lwtheme,
>+#main-window:-moz-has-lwtheme:not([active="true"]) .tabbrowser-tab {
>   color: inherit;
>   text-shadow: inherit;
> }

Both .tabbrowser-tab[selected="true"] and .tabbrowser-tab:hover override .tabbrowser-tab:-moz-has-lwtheme, so you should move this down.
Comment 30 Nochum Sossonko [:Natch] 2009-09-15 07:23:54 PDT
(In reply to comment #27)
> (In reply to comment #26)
> > Everything else was addressed.
> 
> Except for the checkbox changes?

Yeah, meant to comment about that too. I'd like to do that in a different bug, my editor is overloaded with tabs atm :)

Filed bug 516701. I'll update the rest tonight.
Comment 31 Dão Gottwald [:dao] 2009-09-16 09:03:36 PDT
Comment on attachment 400643 [details] [diff] [review]
patch ver. 1.3

oops...
Comment 32 Nochum Sossonko [:Natch] 2009-09-16 16:09:58 PDT
Created attachment 401125 [details] [diff] [review]
patch ver. 1.4

Update to comments.
Comment 33 Dão Gottwald [:dao] 2009-09-17 01:56:43 PDT
Comment on attachment 401125 [details] [diff] [review]
patch ver. 1.4

r=me provided that we find a compelling argument for has-lwtheme (I think we don't have one yet), or use lwtheme, or find a better third option (I'm not crazy about "lwtheme" as an abbreviation, but "lightweighttheme" seems verbose).
Comment 34 Dão Gottwald [:dao] 2009-09-19 02:32:57 PDT
I think we need this for 1.9.2, since inefficient selectors can slow down the UI quite a bit.
Comment 35 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-09-22 15:13:33 PDT
I'm ok with :-moz-lwtheme instead of :-moz-has-lwtheme.
Comment 36 Nochum Sossonko [:Natch] 2009-09-22 16:37:20 PDT
Created attachment 402219 [details] [diff] [review]
patch ver. 1.5

s/-moz-has-lwtheme/-moz-lwtheme/g

Everything else remains the same.
Comment 37 Nochum Sossonko [:Natch] 2009-09-22 16:38:50 PDT
Created attachment 402220 [details] [diff] [review]
patch ver. 1.5

Whoops, wrong patch. Sorry for the bugspam.
Comment 38 Mike Beltzner [:beltzner, not reading bugmail] 2009-09-30 07:49:06 PDT
dbaron: can we get a quick review of the patch with the change you asked for?
Comment 39 Nochum Sossonko [:Natch] 2009-10-01 17:13:49 PDT
Is this really a beta-blocker? If so, I won't be available starting tomorrow, October 2 ~500PM EST, to update the patch in time for the freeze, so if review can be done sometime today I'd be able to update the patch still. Otherwise if there are any minor nits, can someone fill them in for me and land this?

(In reply to comment #38)
> dbaron: can we get a quick review of the patch with the change you asked for?

Are there any pending comments/changes that are expected from the current patch? I'm not aware of any...
Comment 40 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-10-01 17:49:01 PDT
Comment on attachment 402220 [details] [diff] [review]
patch ver. 1.5

nsIDocument.h:

+  virtual int GetDocumentLWTheme() { return Doc_Theme_Uninitialized; }

Seems like you should return Doc_Theme_None from nsIDocument.
(nsXULDocument can still initialize to Uninitialized.)

+  /**
+   * Returns Doc_Theme_Uninitialized if the document lightweight theme is
+   * uninitialized, Doc_Theme_None if there is no lwtheme specified, Doc_Theme_Dark
+   * for a dark theme and Doc_Theme_Bright for a light theme. This is used to
+   * determine the state of the pseudoclasses :-moz-lwtheme and :-moz-lwtheme-text.
+   */

The comments shouldn't allow returning Uninitialized; this method
should always return one of the other values.


In nsCSSRuleProcessor.cpp:
+      nsIDocument* doc = data.mContent ? data.mContent->GetDocument() :
+                                         data.mPresContext->Document();

the GetDocument calls on the data.mContent should be GetOwnerDoc()
instead, and I don't think you should bother with getting the document
from the pres context otherwise.

+  // check for the lwtheme and lwthemetextcolor attribute on root XUL elements
+  if ((aData->mAttribute == nsGkAtoms::lwtheme ||
+       aData->mAttribute == nsGkAtoms::lwthemetextcolor) &&
+      aData->mNameSpaceID == kNameSpaceID_XUL &&
+      aData->mContent == aData->mContent->GetOwnerDoc()->GetRootContent())
+  {
+    data.change = nsReStyleHint(data.change | eReStyle_Self);
+  }

This should be merged with the localedir check right above it.

r=dbaron with those things fixed
Comment 41 Nochum Sossonko [:Natch] 2009-10-01 20:29:49 PDT
Created attachment 404179 [details] [diff] [review]
patch ver. 1.6

Thanks! I addresses all the comments.
Comment 42 Dão Gottwald [:dao] 2009-10-01 23:31:35 PDT
http://hg.mozilla.org/mozilla-central/rev/345a62043ffc
Comment 44 Nochum Sossonko [:Natch] 2009-10-02 12:01:57 PDT
Thanks Dao.
Comment 46 Nochum Sossonko [:Natch] 2009-10-05 21:13:54 PDT
(In reply to comment #45)
The attributes all of these css selectors depend on are:

lwtheme="true"
lwthemetextcolor=["bright"|"dark"]

Afaik, there isn't any "lightweightthemes" attribute.
Comment 47 Dão Gottwald [:dao] 2009-10-06 01:31:11 PDT
(In reply to comment #46)
> The attributes all of these css selectors depend on are:
> 
> lwtheme="true"
> lwthemetextcolor=["bright"|"dark"]

This is an implementation detail and should not be documented.

> Afaik, there isn't any "lightweightthemes" attribute.

There is.

(In reply to comment #45)
> https://developer.mozilla.org/en/XUL/Attribute/lwthemetextcolor

Sheppy, could you please remove this page? I've already removed the references to it.
Comment 48 Daniel Holbert [:dholbert] (largely AFK until June 28) 2009-10-13 16:11:11 PDT
Created attachment 406124 [details] [diff] [review]
follow-up: reorder initializers in constructor, to fix build warning

This bug's checkin introduced this build warning:
{
/mozilla/content/xul/document/src/nsXULDocument.h: In constructor ‘nsXULDocument::nsXULDocument()’:
/mozilla/content/xul/document/src/nsXULDocument.h:485: warning: ‘nsXULDocument::mResolutionPhase’ will be initialized after
/mozilla/content/xul/document/src/nsXULDocument.h:353: warning:   ‘nsIDocument::DocumentTheme nsXULDocument::mDocLWTheme’
/mozilla/content/xul/document/src/nsXULDocument.cpp:224: warning:   when initialized here
}

Attached patch fixes this.
Comment 49 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2009-10-18 18:52:36 PDT
Comment on attachment 406124 [details] [diff] [review]
follow-up: reorder initializers in constructor, to fix build warning

r=dbaron
Comment 50 Daniel Holbert [:dholbert] (largely AFK until June 28) 2009-10-18 20:11:52 PDT
pushed follow-up warning fix:
http://hg.mozilla.org/mozilla-central/rev/7b0e65cf4226

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