Closed
Bug 693461
Opened 13 years ago
Closed 13 years ago
Make inContentUI.css work for XHTML pages
Categories
(Toolkit :: Themes, defect)
Toolkit
Themes
Tracking
()
RESOLVED
FIXED
mozilla10
People
(Reporter: philikon, Assigned: philikon)
References
Details
Attachments
(1 file, 3 obsolete files)
11.38 KB,
patch
|
dao
:
review+
|
Details | Diff | Splinter Review |
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 | ||
Updated•13 years ago
|
Assignee: nobody → philipp
Assignee | ||
Comment 1•13 years ago
|
||
Here's a WIP, just for Windows before I change all the other files.
Attachment #566103 -
Flags: feedback?(bmcbride)
Comment 2•13 years ago
|
||
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+
Assignee | ||
Comment 3•13 years ago
|
||
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;
}
Assignee | ||
Comment 4•13 years ago
|
||
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 5•13 years ago
|
||
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-
Assignee | ||
Comment 6•13 years ago
|
||
(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.
Assignee | ||
Comment 7•13 years ago
|
||
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 8•13 years ago
|
||
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?
Assignee | ||
Comment 9•13 years ago
|
||
(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 10•13 years ago
|
||
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-
Assignee | ||
Comment 11•13 years ago
|
||
(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!
Assignee | ||
Comment 12•13 years ago
|
||
Attachment #567306 -
Attachment is obsolete: true
Attachment #567659 -
Flags: review?(dao)
Updated•13 years ago
|
Attachment #567659 -
Flags: review?(dao) → review+
Comment 13•13 years ago
|
||
landed on s-c . https://tbpl.mozilla.org/?tree=Services-Central&rev=cf2b4e7d9103
Updated•13 years ago
|
Whiteboard: [fixed-in-services]
Assignee | ||
Comment 14•13 years ago
|
||
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.
Description
•