693461 - Make inContentUI.css work for XHTML pages

Status: RESOLVED FIXED

(Toolkit :: Themes, defect)

Description

[Philipp von Weitershausen [:philikon]]

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.

Comment 1

[Philipp von Weitershausen [:philikon]]

Attachment: wip (winstripe only)

Here's a WIP, just for Windows before I change all the other files.

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

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...).

Comment 3

[Philipp von Weitershausen [:philikon]]

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;
  }

Comment 4

[Philipp von Weitershausen [:philikon]]

Attachment: v1

Here's a patch for all themes with Blair's feedback applied and the rule for the default font mentioned in comment 3.

Comment 5

[Dão Gottwald [: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.

Comment 6

[Philipp von Weitershausen [:philikon]]

(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.

Comment 7

[Philipp von Weitershausen [:philikon]]

Attachment: v2

Use XUL as default namespace and prefix rules applicable to XUL and HTML with *|.

Comment 8

[Dão Gottwald [: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?

Comment 9

[Philipp von Weitershausen [:philikon]]

(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

[Dão Gottwald [:dao]]

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.

Comment 11

[Philipp von Weitershausen [:philikon]]

(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!

Comment 12

[Philipp von Weitershausen [:philikon]]

Attachment: v3

Comment 13

[Allison Naaktgeboren :ally]

landed on s-c . https://tbpl.mozilla.org/?tree=Services-Central&rev=cf2b4e7d9103

Comment 14

[Philipp von Weitershausen [:philikon]]

https://hg.mozilla.org/mozilla-central/rev/0e9e8433d5f0