Closed
Bug 513461
Opened 15 years ago
Closed 15 years ago
Need pseudo classes for lightweight theme handling
Categories
(Core :: CSS Parsing and Computation, defect, P1)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla1.9.3a1
Tracking | Status | |
---|---|---|
status1.9.2 | --- | beta1-fixed |
People
(Reporter: dao, Assigned: Natch)
References
Details
(Keywords: dev-doc-complete, perf)
Attachments
(2 files, 9 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
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".
Assignee | ||
Comment 1•15 years ago
|
||
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.
Assignee: nobody → highmind63
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•15 years ago
|
||
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.
Reporter | ||
Comment 4•15 years ago
|
||
Nochum, would you also like to update the stylesheets that use the attributes or should I prepare that patch?
Comment 5•15 years ago
|
||
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)
Reporter | ||
Comment 6•15 years ago
|
||
-moz-lwtheme-is(dark) is wrong. This is about the text color, hence the proposed :-moz-lwtheme-darktext and :-moz-lwtheme-brighttext.
Assignee | ||
Comment 7•15 years ago
|
||
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?
Reporter | ||
Comment 8•15 years ago
|
||
(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.
Assignee | ||
Comment 9•15 years ago
|
||
Patch ready for review, including the parsing test and changed the names in accordance with the comments. The stylesheets have been updated as well.
Attachment #398578 -
Attachment is obsolete: true
Attachment #398980 -
Flags: superreview?(dbaron)
Attachment #398980 -
Flags: review?(dbaron)
Assignee | ||
Comment 10•15 years ago
|
||
Comment on attachment 398980 [details] [diff] [review] patch Dao, can you review the stylesheet changes?
Attachment #398980 -
Flags: review?(dao)
Assignee | ||
Comment 11•15 years ago
|
||
Sorry for the bugspam, patch was full of cruft from earlier attempts and some line-endings were changed unintentionally.
Attachment #398980 -
Attachment is obsolete: true
Attachment #398982 -
Flags: superreview?(dbaron)
Attachment #398982 -
Flags: review?(dbaron)
Attachment #398980 -
Flags: superreview?(dbaron)
Attachment #398980 -
Flags: review?(dbaron)
Attachment #398980 -
Flags: review?(dao)
Assignee | ||
Updated•15 years ago
|
Attachment #398982 -
Flags: review?(dao)
Reporter | ||
Comment 13•15 years ago
|
||
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.
Assignee | ||
Comment 14•15 years ago
|
||
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.
Reporter | ||
Comment 15•15 years ago
|
||
(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.)
Reporter | ||
Comment 16•15 years ago
|
||
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.)
Assignee | ||
Comment 17•15 years ago
|
||
This patch addresses all of Dao's previous comments.
Attachment #398982 -
Attachment is obsolete: true
Attachment #399264 -
Flags: superreview?(dbaron)
Attachment #399264 -
Flags: review?(dbaron)
Attachment #398982 -
Flags: superreview?(dbaron)
Attachment #398982 -
Flags: review?(dbaron)
Attachment #398982 -
Flags: review?(dao)
Assignee | ||
Updated•15 years ago
|
Attachment #399264 -
Flags: review?(dao)
Reporter | ||
Comment 18•15 years ago
|
||
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.)
Reporter | ||
Comment 19•15 years ago
|
||
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.
Attachment #399264 -
Flags: review?(dao) → review-
Assignee | ||
Comment 20•15 years ago
|
||
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.
Reporter | ||
Comment 21•15 years ago
|
||
[_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.
Assignee | ||
Comment 22•15 years ago
|
||
Attachment #399264 -
Attachment is obsolete: true
Attachment #399362 -
Flags: superreview?(dbaron)
Attachment #399362 -
Flags: review?(dbaron)
Attachment #399264 -
Flags: superreview?(dbaron)
Attachment #399264 -
Flags: review?(dbaron)
Assignee | ||
Updated•15 years ago
|
Attachment #399362 -
Flags: review?(dao)
Reporter | ||
Comment 23•15 years ago
|
||
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?
Attachment #399362 -
Flags: review?(dao) → review-
Reporter | ||
Comment 24•15 years ago
|
||
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.
Reporter | ||
Comment 25•15 years ago
|
||
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.
Assignee | ||
Comment 26•15 years ago
|
||
(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.
Attachment #399362 -
Attachment is obsolete: true
Attachment #400643 -
Flags: superreview?(dbaron)
Attachment #400643 -
Flags: review?(dbaron)
Attachment #399362 -
Flags: superreview?(dbaron)
Attachment #399362 -
Flags: review?(dbaron)
Assignee | ||
Updated•15 years ago
|
Attachment #400643 -
Flags: review?(dao)
Reporter | ||
Comment 27•15 years ago
|
||
(In reply to comment #26) > Everything else was addressed. Except for the checkbox changes?
Reporter | ||
Comment 28•15 years ago
|
||
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.
Reporter | ||
Comment 29•15 years ago
|
||
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.
Assignee | ||
Comment 30•15 years ago
|
||
(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.
Reporter | ||
Updated•15 years ago
|
Attachment #400643 -
Flags: review?(dbaron)
Reporter | ||
Comment 31•15 years ago
|
||
Comment on attachment 400643 [details] [diff] [review] patch ver. 1.3 oops...
Attachment #400643 -
Flags: review?(dao) → review?(dbaron)
Assignee | ||
Comment 32•15 years ago
|
||
Update to comments.
Attachment #400643 -
Attachment is obsolete: true
Attachment #401125 -
Flags: superreview?(dbaron)
Attachment #401125 -
Flags: review?(dbaron)
Attachment #401125 -
Flags: review?(dao)
Attachment #400643 -
Flags: superreview?(dbaron)
Attachment #400643 -
Flags: review?(dbaron)
Reporter | ||
Comment 33•15 years ago
|
||
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).
Attachment #401125 -
Flags: review?(dao) → review+
Reporter | ||
Comment 34•15 years ago
|
||
I think we need this for 1.9.2, since inefficient selectors can slow down the UI quite a bit.
Flags: blocking1.9.2?
Flags: blocking1.9.2? → blocking1.9.2+
Priority: -- → P1
Assignee | ||
Comment 36•15 years ago
|
||
s/-moz-has-lwtheme/-moz-lwtheme/g Everything else remains the same.
Attachment #401125 -
Attachment is obsolete: true
Attachment #402219 -
Flags: superreview?(dbaron)
Attachment #402219 -
Flags: review?(dbaron)
Attachment #401125 -
Flags: superreview?(dbaron)
Attachment #401125 -
Flags: review?(dbaron)
Assignee | ||
Comment 37•15 years ago
|
||
Whoops, wrong patch. Sorry for the bugspam.
Attachment #402219 -
Attachment is obsolete: true
Attachment #402220 -
Flags: superreview?(dbaron)
Attachment #402220 -
Flags: review?(dbaron)
Attachment #402219 -
Flags: superreview?(dbaron)
Attachment #402219 -
Flags: review?(dbaron)
Comment 38•15 years ago
|
||
dbaron: can we get a quick review of the patch with the change you asked for?
Reporter | ||
Updated•15 years ago
|
Keywords: dev-doc-needed
Assignee | ||
Comment 39•15 years ago
|
||
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•15 years ago
|
||
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
Attachment #402220 -
Flags: superreview?(dbaron)
Attachment #402220 -
Flags: review?(dbaron)
Attachment #402220 -
Flags: review+
Assignee | ||
Comment 41•15 years ago
|
||
Thanks! I addresses all the comments.
Attachment #402220 -
Attachment is obsolete: true
Assignee | ||
Updated•15 years ago
|
Keywords: checkin-needed
Reporter | ||
Comment 42•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/345a62043ffc
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Reporter | ||
Comment 43•15 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/ce3da669a2dd
status1.9.2:
--- → beta1-fixed
Keywords: checkin-needed
Comment 45•15 years ago
|
||
Now documented: https://developer.mozilla.org/en/CSS/:-moz-lwtheme https://developer.mozilla.org/en/CSS/:-moz-lwtheme-brighttext https://developer.mozilla.org/en/CSS/:-moz-lwtheme-darktext https://developer.mozilla.org/en/XUL/Attribute/lwthemetextcolor And the related article: https://developer.mozilla.org/en/Themes/Lightweight_themes
Keywords: dev-doc-needed → dev-doc-complete
Assignee | ||
Comment 46•15 years ago
|
||
(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.
Reporter | ||
Comment 47•15 years ago
|
||
(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•15 years ago
|
||
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.
Updated•15 years ago
|
Attachment #406124 -
Attachment description: follow-up warning fix → follow-up: reorder initializers in constructor, to fix build warning
Attachment #406124 -
Flags: review?(dbaron)
Attachment #406124 -
Flags: approval1.9.2?
Updated•15 years ago
|
Attachment #406124 -
Flags: approval1.9.2?
Comment 49•15 years ago
|
||
Comment on attachment 406124 [details] [diff] [review] follow-up: reorder initializers in constructor, to fix build warning r=dbaron
Attachment #406124 -
Flags: review?(dbaron) → review+
Comment 50•15 years ago
|
||
pushed follow-up warning fix: http://hg.mozilla.org/mozilla-central/rev/7b0e65cf4226
You need to log in
before you can comment on or make changes to this bug.
Description
•