1581914 - Set default margins for html|input in global.css

Status: RESOLVED FIXED

(Toolkit :: Themes, task, P2)

Description

[Brian Grinstead [:bgrins]]

After speaking with Tim and Dão, we decided that we will try to set the default margin for all html inputs to match the margins we currently set for xul|textbox, xul|label, xul|checkbox, xul|radio, etc.

This pro here is that it will make the migration work in Bug 1547372 more straightforward. We could continue to convert <textbox> elements without overriding each individual consumer's margin to match.

The cons are that this will apply to all inputs in docs that load global.css which:

1. Will apply new styles that may need to be overridden for docs that have existing html inputs and also load global.css (for instance, devtools, createProfileWizard, various tests)
2. Will apply to all inputs and not just textboxes. So places using [type="check"], [type="radio"] etc will also need (1). We could alternatively apply this to just input[type="text"] but that will increase the specificity of the default and make it harder to override by a class (which would then require !important or increasing specificity to override).

Another option we considered was adding a class to opt in to the margin, which would be safer for (1) and (2) but if we ended up needing the class on the majority of our form elements then it'd be easier for a default to do the work.

Comment 1

[Tim Nguyen :ntim]

FWIW, Thunderbird is also converting textboxes to inputs (bug 1562161) and they use an opt-in class: https://searchfox.org/comm-central/search?q=input-fields.css&case=false&regexp=false&path=

Comment 2

[Tim Nguyen :ntim]

Some of the styling that was added for previous textbox conversions could be reverted if we go with this:

bug 1562809, bug 1562652, bug 1562240

Comment 3

[Brian Grinstead [:bgrins]]

Attachment: Bug 1581914 - WIP append the devtools theme stylesheet sheet to document.head if it exists instead of using a PI

Otherwise, what happens in the webconsole document is the theme file gets loaded before
global.css, which makes overriding the styles from global.css difficult.

Comment 4

[Brian Grinstead [:bgrins]]

Attachment: Bug 1581914 - Set default margins for html|input in global.css

Depends on D46530

Comment 5

[Brian Grinstead [:bgrins]]

Attachment: browser-console-nightly.png

Comment 6

[Brian Grinstead [:bgrins]]

Attachment: browser-console-with-defaults.png

Overriding the defaults from the webconsole (which loads global.css as a <link> tag since the browser console is a top-level window) is surfacing some issues.

1. We are currently appending the devtools theme sheet from JS as an XML PI which means that it gets loaded before global.css in the document. This means a rule on just input can't override the global.css default. I've put up a patch that fixes this (append the theme as a link after global.css). However, this is leading the buttons to become shrunken for some reason unrelated to the default margin change.
2. input type=check inputs do have a default UA style. Setting margin: initial doesn't restore this leading to the text being too close to the checkbox. I think margin: revert is what we actually need here.

Comment 7

[Dão Gottwald [:dao]]

(In reply to Brian Grinstead [:bgrins] from comment #6)

1. We are currently appending the devtools theme sheet from JS as an XML PI which means that it gets loaded before global.css in the document. This means a rule on just inputt can't override the global.css default. I've put up a patch that fixes this (append the theme as a link after global.css). [...]

Ugh, this should be fixed regardless of this bug if devtools wants to keep loading global.css going forward. (Does it, though?)

Comment 8

[Brian Grinstead [:bgrins]]

(In reply to Dão Gottwald [::dao] from comment #7)

(In reply to Brian Grinstead [:bgrins] from comment #6)

1. We are currently appending the devtools theme sheet from JS as an XML PI which means that it gets loaded before global.css in the document. This means a rule on just inputt can't override the global.css default. I've put up a patch that fixes this (append the theme as a link after global.css). [...]

Ugh, this should be fixed regardless of this bug if devtools wants to keep loading global.css going forward. (Does it, though?)

Yes, at least for the toolbox, browser toolbox, and webconsole documents. All of them are supported as top level windows which need to host context menus, panels, etc (which rely on styles in global and widgets.css).

This requirement could go away if we moved all of the relevant styling into the shadow dom tree instead (using :host instead of i.e. panel to provide global styling), but that's a big project and not currently prioritized.

Comment 9

[Phabricator Automation]

Comment on attachment 9094039 [details]
Bug 1581914 - WIP append the devtools theme stylesheet sheet to document.head if it exists instead of using a PI

Revision D46530 was moved to bug 1582786. Setting attachment 9094039 [details] to obsolete.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/58d3b17e4f8c
Set default margins for html|input in global.css r=dao

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/58d3b17e4f8c