Closed Bug 693461 Opened 9 years ago Closed 9 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]
https://hg.mozilla.org/mozilla-central/rev/0e9e8433d5f0
Status: NEW → RESOLVED
Closed: 9 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.