1686613 - In the new non-native widget theme, if I specify padding for one side, all other sides reset to have 0 padding

Status: RESOLVED FIXED

(Core :: Widget, defect)

Description

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

STR:

1. Run Firefox with about:config pref widget.disable-native-theme-for-content set to true.
2. Load attached testcase.

EXPECTED RESULTS:
Each pair of form controls should look the same (horizontally at least; their height / y-positioning might vary slightly)

ACTUAL RESULTS:
In the second and fourth form control, the contents are shifted to the left, and the form control is skinnier than its partner. (This is really just a symptom of its padding properties having been reset to 0 for some reason, aside from the one padding-top property that was specified explicitly on that element.)

Comment 1

[Daniel Holbert [:dholbert]]

Firefox release (without this pref set) and Chrome both give EXPECTED RESULTS here, btw.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: screenshot of bug

Comment 3

[Emilio Cobos Álvarez (:emilio)]

This is somewhat expected, see bug 1624696.

We should either change the UA sheet somehow when non-native theme is enabled, and remove the widget specified padding altogether, or make the AUTHOR_SPECIFIED_PADDING flags more granular, but I'd rather not do the later.

Comment 4

[Chris Peterson [:cpeterson]]

Tracking non-native theming bugs for Fission Beta milestone (M7).

Comment 5

[Cameron McCormack (:heycam)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)

We should either change the UA sheet somehow when non-native theme is enabled, and remove the widget specified padding altogether

One slight annoyance with that is that we'd need different UA sheets for the parent and content processes (or some new MQ which checked the non-native theme pref and what type of process we're in).

Comment 6

[Stephen A Pohl [:spohl]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)

This is somewhat expected, see bug 1624696.

We should either change the UA sheet somehow when non-native theme is enabled, and remove the widget specified padding altogether, or make the AUTHOR_SPECIFIED_PADDING flags more granular, but I'd rather not do the later.

Given comment 5, do you still prefer the first approach? What are the downsides of doing the latter? Adding granularity seems more straightforward, but I'm sure there are good reasons why this isn't the preferred approach.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Adding granularity is somewhat annoying and not too expected, imo.

(In reply to Cameron McCormack (:heycam) from comment #5)

One slight annoyance with that is that we'd need different UA sheets for the parent and content processes (or some new MQ which checked the non-native theme pref and what type of process we're in).

This seems not too hard to implement, IMO. I can poke in a bit.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1686613 - Add a non-native-theme media query. r=spohl

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1686613 - Use a consistent style for attribute selectors in forms.css. r=spohl

Depends on D102457

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1686613 - Port non-native theme paddings to forms.css. r=spohl

The only change I've made intentionally is the change to the textarea /
input padding. Now that the focus outline doesn't cover the padding
area, this preserves the same visual effect, plus the 2px inline padding
values now match Chrome, which means that even affecting appearance:
none controls, this is probably compatible.

Depends on D102458

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/051720b8cf72
Use a consistent style for attribute selectors in forms.css. r=spohl

Comment 12

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/051720b8cf72

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/80240fe58a75
Add a non-native-theme media query. r=heycam

Comment 14

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/80240fe58a75

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9983175adb47
Port non-native theme paddings to forms.css. r=mstange

Comment 16

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/9983175adb47