Closed Bug 693461 Opened 13 years ago Closed 13 years ago

Make inContentUI.css work for XHTML pages

Categories

(Toolkit :: Themes, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla10

People

(Reporter: philikon, Assigned: philikon)

References

Details

Attachments

(1 file, 3 obsolete files)

We want to create in-content UI with XHTML (see bug 675822) so it'd be nice if inContentUI.css didn't use XUL for the default namespace. I suggest defining XHTML and XUL namespaces and prefixing the current rules for XUL elements. That way we can use the class rules for :root and .main-content in XHTML and also add rules for XHTML elements later on.
Assignee: nobody → philipp
Attached patch wip (winstripe only) (obsolete) — Splinter Review
Here's a WIP, just for Windows before I change all the other files.
Attachment #566103 - Flags: feedback?(bmcbride)
Comment on attachment 566103 [details] [diff] [review] wip (winstripe only) Review of attachment 566103 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/themes/winstripe/global/inContentUI.css @@ +134,4 @@ > outline: 1px dotted ThreeDDarkShadow; > } > > + html|button[disabled="disabled"], Fun fact: HTML lets you use *any* value for the disabled attribute (even disabled="false" will make it disabled). So you'll want html|button[disabled] here and elsewhere. @@ +154,5 @@ > border-color: rgba(39, 53, 68, 0.5); > box-shadow: 0 0 3px 1px rgba(39, 53, 68, 0.2) inset; > } > > button > .button-box { This should probably be prefixed too, since HTML buttons don't have the anonymous content that XUL (XBL) buttons do (unless the author adds a <div> as a child...).
Attachment #566103 - Flags: feedback?(bmcbride) → feedback+
Thanks for the feedback, Blair! We probably also want to add a rule to set the default font in HTML: html|html { font: message-box; }
Attached patch v1 (obsolete) — Splinter Review
Here's a patch for all themes with Blair's feedback applied and the rule for the default font mentioned in comment 3.
Attachment #566103 - Attachment is obsolete: true
Attachment #566301 - Flags: review?(dao)
Comment on attachment 566301 [details] [diff] [review] v1 >--- a/toolkit/themes/pinstripe/global/inContentUI.css >+++ b/toolkit/themes/pinstripe/global/inContentUI.css >+html|button[disabled], > button[disabled="true"], > menulist[disabled="true"], > colorpicker[type="button"][disabled="true"] { Please add xul| to the XUL-targeting selectors. > .spinbuttons-button > .button-box > .button-text { > display: none; > } > > .spinbuttons-button[disabled="true"] > .button-box > .button-icon { > opacity: 0.5; > } ditto Alternatively, you could keep XUL as the default namespace and use *| for rules targeting HTML and XUL.
Attachment #566301 - Flags: review?(dao) → review-
(In reply to Dão Gottwald [:dao] from comment #5) > Alternatively, you could keep XUL as the default namespace and use *| for > rules targeting HTML and XUL. Thanks, Dão. That might be easier and a smaller diff. Will do.
Attached patch v2 (obsolete) — Splinter Review
Use XUL as default namespace and prefix rules applicable to XUL and HTML with *|.
Attachment #566301 - Attachment is obsolete: true
Attachment #567306 - Flags: review?(dao)
Comment on attachment 567306 [details] [diff] [review] v2 >--- a/toolkit/themes/pinstripe/global/inContentUI.css >+++ b/toolkit/themes/pinstripe/global/inContentUI.css > button:-moz-focusring > .button-box, > menulist:-moz-focusring:not([open="true"]) > .menulist-label-box, > colorpicker[type="button"]:-moz-focusring:not([open="true"]) > .colorpicker-button-colorbox { > outline: 1px dotted #252F3B; > } Do you need something similar for HTML buttons?
(In reply to Dão Gottwald [:dao] from comment #8) > > button:-moz-focusring > .button-box, > > menulist:-moz-focusring:not([open="true"]) > .menulist-label-box, > > colorpicker[type="button"]:-moz-focusring:not([open="true"]) > .colorpicker-button-colorbox { > > outline: 1px dotted #252F3B; > > } > > Do you need something similar for HTML buttons? HTML buttons seem to be getting the dotted line to indicate focus for free, so it appears I don't.
Comment on attachment 567306 [details] [diff] [review] v2 >--- a/toolkit/themes/gnomestripe/global/inContentUI.css >+++ b/toolkit/themes/gnomestripe/global/inContentUI.css >-:root { >+*|:root { >-.main-content { >+*|.main-content { Neither of these changes work. You need to use *|* if you don't care about the node name.
Attachment #567306 - Flags: review?(dao) → review-
(In reply to Dão Gottwald [:dao] from comment #10) > >-.main-content { > >+*|.main-content { > > Neither of these changes work. You need to use *|* if you don't care about > the node name. Gah, you're right. I thought I had tested it, but it turns out the changes weren't picked up by the incremental build for some reason. New patch coming up!
Attached patch v3Splinter Review
Attachment #567306 - Attachment is obsolete: true
Attachment #567659 - Flags: review?(dao)
Attachment #567659 - Flags: review?(dao) → review+
Whiteboard: [fixed-in-services]
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-services]
Target Milestone: --- → mozilla10
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: