244058 - BUTTON elements forced to native look even with CSS unsupported by native theme (e.g. background)

Status: RESOLVED FIXED

(Camino Graveyard :: HTML Form Controls, defect)

Description

[Stuart Morgan]

<button> elements are always drawn native, but the consensus seems to be that we
should revert to gfx for styled <buttons> since they can be way more complex
than a simple <input> button. This will let us support background images and
colors for <button>s so buttons with light text colors are still readable (see
travelocity link).

I'll hopefully have a fix soon to add NS_BUTTON_BEVEL support to IsWidgetStyled.

Comment 1

[Stuart Morgan]

CCing pinkerton and bryner, since they'll have to approve it at some point.

Comment 2

[Stuart Morgan]

Attachment: Disables <button> theme support

After messing around with this and considering, my opinion is that we should
always use gfx-style <button>s. They can contain arbitrary HTML, so there's no
good way of knowing if styled buttons make sense. If we use IsWidgetStyled,
we'll get weird, inconsisntant results. Example:

<button style="background-color:red; color:blue"><div>Click</div></button>
<button><div style="background-color:red; color:blue">Click</div></button>

These should look basically the same, but any use of styling detection for
theming will give us one themed button and one unthemed. Since there's no good
way to tell, and I think we should have both consistency and stylability, I
vote for gfx-buttons (especially since I don't think most people would use a
<button> over an <input> if they weren't doing semi-complex things with them).

Comment 3

[Stuart Morgan]

Nominating for 0.8, since I really think we need to do *something* to make sure
we aren't allowing foreground w/o background on <button>s. Otherwise, sites with
such buttons (like travelocity) are unusable.

Comment 4

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 149109 [details] [diff] [review]
Disables <button> theme support

r=pink.

Comment 5

[Mike Pinkerton (not reading bugmail)]

Attachment: better patch

instead of removing all code for bevel buttons in the theme code, just don't
use -moz-appearance in css for <button> tag

Comment 6

[Stuart Morgan]

Hm... I thought I tried that and it made <button>s use NS_THEME_BUTTON instead
of BUTTON_BEVEL, and thus get themed sometimes.

Comment 7

[Stuart Morgan]

Yeah, I just double-checked. Pink's patch will make <button>s into
NS_THEME_BUTTONs, and will thus be themed if unstyled (and will in fact go from
themed when enabled to gfx when disabled, which is extra bad).

Comment 8

[Stuart Morgan]

The better and working patch is to replace
  -moz-appearance: button-bevel;
  margin: 5px 2px 0px 2px;
  padding: 0px;
  border-width: 2px 4px 4px 4px;
With
  -moz-appearance: none;

I don't think we want to get rid of the line for moz-focus-inner, 'cause that's
just ugly. We still get feedback from the button-push.

No source tree handy at the moment, so I can't attach a new patch, but I tested
this and it works.

Comment 9

[Stuart Morgan]

Attachment: working better patch

Always makes <button> un-themed, adds the moz-focus-ring back in for <button>s

Comment 10

[Stuart Morgan]

Comment on attachment 149584 [details] [diff] [review]
working better patch

We presumably need an a= for the 1.7 branch as well, so please request that
from the appropriate person.

Comment 11

[Asa Dotzler [:asa]]

Comment on attachment 149584 [details] [diff] [review]
working better patch

a=asa (on behalf of drivers) for checkin to 1.7

Comment 12

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 149584 [details] [diff] [review]
working better patch

r=pink

Comment 13

[Mike Pinkerton (not reading bugmail)]

landed on trunk and branch