309556 - html 'button' element not drawn like 'input type=button' (needs Aqua styles)

Status: RESOLVED FIXED

(Camino Graveyard :: HTML Form Controls, defect)

Description

[glenn.jackman]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b4) Gecko/20050914 Camino/1.0a1
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b4) Gecko/20050914 Camino/1.0a1

An 'input' button is drawn in the aqua-style
a 'button' element is drawn in the firefox style (square)

Reproducible: Always

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

See also bug 205136 on unwanted style rules on form buttons (doesn't look like
Stuart ever filed the bug on styling <button>).

Comment 2

[Stuart Morgan]

Just so you have the historical context, this was done deliberately (bug
244058).  They used to be Aqua, but that rendered the buttons on some sites
(like travelocity) completely unusable because of issues like bug 244058.

You can see bug 244058 for my rational in making the change (after disucussions
with pink). Basically, because <button>s can contain arbitrary HTML, the options
I saw were:
1) Always use gfx-style
2) Use a very simple heuristic (the old way) and be inconsistant and sometimes
unusably wrong
3) Use a very complex heuristic based on all of the contents of the button
element and what their stylings are (keeping in mind things like the potential
for the contents to change dynamically though JS).

1 was deemed better than 2, thus the change (especially since at the time Google
wasn't using them for normalish buttons, so examples in the wild were generally
styled enough to look better as styled gfx than Aquaish).  3 is, as you can see,
distictly non-trivial.

Comment 3

[Stuart Morgan]

Sorry, first paragraph should say "because of issues like bug 205136"

Comment 4

[Mike Pinkerton (not reading bugmail)]

so this is WONTFIX?

Comment 5

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I think a simpler heuristic than the old, broken-and-now-removed option 2 would
improve button appearance in a lot of common cases: 

If a <button> contains styles *other* than plain text, bold text, or italic
text, use gfx <button>.  For those three cases, use Aqua and style the text (as
we do now for input buttons.  

We're starting to see lots of <buttons> on common sites that just have text,
perhaps bold text, and they look ugly, esp. if there are input buttons or other
Aqua widgets on the page, too (Gmail, Yahoo Mail off the top of my head).

I'm not sure how simple this is in code, but I think it's preferable to WONTFIX :-)

Comment 6

[Stuart Morgan]

(In reply to comment #5)
> If a <button> contains styles *other* than plain text, bold text, or italic
> text, use gfx <button>.  For those three cases, use Aqua and style the text (as
> we do now for input buttons.  

That's an big oversimplification. (I'll stop chiming in now, I promise--It's
just that I thought about this a lot when I looked at this back in the day, and
figure it's worth recording what I had considered.)  Any overly-simple algorithm
will cause wild inconsistencies.  If you just whitelist based on those three
styles, what if a button has text with a pixel of padding on it?  Boom, suddenly
it's a completely different shape, size, and look.  So maybe you whitelist
padding.  And margin.  What if the text has (or has inherited) some word-wrap,
alignment, or white-space style?  So maybe you whitelist those.  How many other
tiny, almost-impossible-for-designers-to-predict changes would radically alter
the button?

And then there's the other side.  What if it's just unstyled text, but it's got
several <p>s?  Or it's a list?  That's a weird-looking aqua button you'll end up
with.

I'm not trying to say this should or shouldn't be fixed... just be aware that if
you 'fix' it with a very simple algorithm, you may be opening a pretty
substantial can of worms.

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I'm not sure I follow this:

> If you just whitelist based on those three
> styles, what if a button has text with a pixel of padding on it?  Boom, suddenly
> it's a completely different shape, size, and look.  So maybe you whitelist
> padding.

Why would you whitelist padding?  (I think I'd whitelist margin before I'd
whitelist padding, since margin affects the area outside the <button>, not the
stuff inside it.)

That said, I appreciate hearing the thought process you used and the issues--but
since I'm not a programmer, maybe *I* should shut up :-)

Comment 8

[Stuart Morgan]

On the other hand, Hyatt disagrees with me and is smarter than I am ;)
http://webkit.opendarwin.org/blog/?p=28

Comment 9

[Samuel Sidler (old account; do not CC)]

Also see bug 205136.

Comment 10

[Stuart Morgan]

With all the widget work happening on trunk it doesn't make sense for us to revisit this before 2.0.

Comment 13

[Stuart Morgan]

Attachment: fix

This is Smokey's patch from bug 309556.

Since we'll get fall-over behavior on trunk, the serious concerns, so we should do this and worry about how to improve the aesthetics of aqua buttons with arbitrary content in later bugs.

Comment 14

[Stuart Morgan]

Glah, I can't seem to actually do anything right in bugzilla tonight. That should have read "the serious concerns are gone".

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

What do you mean by fallover behavior?  Do the aqua buttons that you're drawing at least grow in both dimensions to contain their content?

Comment 16

[Stuart Morgan]

<button>s will follow all of the new logic that Josh has been writing for button behaviors, which gives them substantially more size flexibility than they have previously had in Camino. By fall-over I just mean that they will follow IsWidgetStyled now, and revert to more styleable base widgets as necessary, rather than having parts of their style unconditionally stomped by !important rules that we used to have in our CSS.

This is just removing one of Camino's deviations from mainline trunk widgets that isn't necessary anymore thanks to the rewrite.

Comment 17

[Stuart Morgan]

Comment on attachment 261920 [details] [diff] [review]
fix

Josh's patch in bug 381539 is a superset of this, so canceling sr?.

Comment 18

[Stuart Morgan]

Fixed by bug 381539