1246836 - -moz-appearance button should behave like -webkit-appearance: button

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Karl Dubost💡 :karlcow]

We have a difference of behavior with regards to WebKit and Blink when setting the appearance to a button.

I created an example at http://codepen.io/anon/pen/QyJMGx
The screenshot in Firefox and Safari are visible at https://github.com/webcompat/web-bugs/issues/1006#issuecomment-181660121

In a scenario such as:

.webkitimg {
  -webkit-appearance: button;
  background: url(data:image/png;base64,…) no-repeat scroll 100% 50% transparent !important;
    background-size: 7px 9px !important;
}

WebKit, Blink drops the button to show only the image.

Gecko keeps both which creates a repetition of widget controls.

Left a comment on the compat spec too.
https://github.com/whatwg/compat/issues/6

Comment 1

[Karl Dubost💡 :karlcow]

Attachment: firefox-webkit-button.png

Adding the screenshot in Firefox and WebKit on rendering button and background image from http://codepen.io/anon/pen/QyJMGx

Comment 2

[Karl Dubost💡 :karlcow]

See also https://webcompat.com/issues/1005

Comment 3

[Karl Dubost💡 :karlcow]

ok let me be clearer.

http://www.la-grange.net/2017/02/21/appearance/appearance-test-0001.html

Comment 4

[Karl Dubost💡 :karlcow]

I created a test.
http://www.la-grange.net/2017/02/21/appearance/appearance-test-0001.html

The issue is that when we set 

    -moz-appearance: button 

we get a widget with arrows while -webkit-appearance: button deliver a simple button.

So if the site does:

    -webkit-appearance: button;
    -moz-appearance: button;
    background: <image>

The rendering looks broken in Firefox.

and if the site does:

    -webkit-appearance: button;
    -moz-appearance: none;
    appearance: button
    background: <image>

the site will be broken if we unship -moz-appearance: none

Comment 5

[Karl Dubost💡 :karlcow]

seeAlso the discussion on dev.platform
https://groups.google.com/forum/#!topic/mozilla.dev.platform/oZ9cPF8Y1pE

Comment 6

[Mats Palmgren (inactive)]

We should fix this before implementing -webkit-appearance.

Comment 7

[Louis Chang [:louis]]

I think here is the place http://searchfox.org/mozilla-central/source/widget/nsNativeTheme.cpp#345-354 where causes us not painting the button with theme when background is set. But I am still figuring out how to fix it in a reasonable way.

Comment 8

[Louis Chang [:louis]]

Attachment: Bug 1246836 - Fallback to render customized button when -moz-appearance is button with border or background is set.

Review commit: https://reviewboard.mozilla.org/r/197604/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/197604/

Comment 9

[Louis Chang [:louis]]

Attachment: testcae.html

Comment 10

[Louis Chang [:louis]]

Comment on attachment 8926319 [details]
Bug 1246836 - Fallback to render customized button when -moz-appearance is button with border or background is set.

https://reviewboard.mozilla.org/r/197604/#review202846

Hi mats,
Could you give some feedback on the patch? Thanks.

I tried to make nsComboBoxControlFrame add both nsDisplayThemedBackground and nsDisplayBackgroundImage to DisplayList so it can paint out both button with native theme and the background image.

::: widget/nsNativeTheme.cpp:351
(Diff revision 1)
> +    if (aFrame->Type() == LayoutFrameType::ComboboxControl)
> +      return false;
> +  }
> +
>    return (aWidgetType == NS_THEME_NUMBER_INPUT ||
>            aWidgetType == NS_THEME_BUTTON ||

It seems contradict to me here. Currently, we allow author to style button and not paint out the button with native theme when border or background is set. But now we expect to paint out both button and border or background.

Do you have any thought or how we could fix this in other way?

Comment 11

[Louis Chang [:louis]]

Attachment: testcase.html

Comment 12

[Louis Chang [:louis]]

Comment on attachment 8926319 [details]
Bug 1246836 - Fallback to render customized button when -moz-appearance is button with border or background is set.

It seems we should fix at the custom drawing instead of the theme part.

Comment 13

[Louis Chang [:louis]]

Comment on attachment 8926319 [details]
Bug 1246836 - Fallback to render customized button when -moz-appearance is button with border or background is set.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/197604/diff/1-2/

Comment 14

[Louis Chang [:louis]]

Comment on attachment 8926319 [details]
Bug 1246836 - Fallback to render customized button when -moz-appearance is button with border or background is set.

Hi mats,

Could you feedback on the patch? Thanks.

So here is proposal of fixing the button issue by adding a anonymous appearance content and a text content into nsComboboxFrame. We dynamic allocate the appearance content based on the appearance value. And render it when there is no theme but appearance is changed. And the selection will be updated to the text content.
The patch is able to successfully fallback render the customized button when appearance is button and border or background is set. Though, there are still some trivial problems need to solved such as the rendering of customized button (will render a down arrow) and could not render the background image since it is cover by appearance content.

Comment 15

[Louis Chang [:louis]]

Note:
So this solution could only fix the select element with appearance button for addressing the most common webcompat issue...
If this work, maybe we could further add an anonymous content to every frame?

Comment 16

[Mats Palmgren (inactive)]

Comment on attachment 8926319 [details]
Bug 1246836 - Fallback to render customized button when -moz-appearance is button with border or background is set.

This solution seems a bit too complex to solve the problem at hand.
We already have code to suppress the dropdown button for
-moz-appearance:none (added in bug 649849).  I think we could
generalize that by inverting the condition in that code so that it
tests -moz-appearance != menulist instead (assuming this is what
Chrome does).

Comment 17

[Mats Palmgren (inactive)]

Attachment: Render the dropdown button for -moz-appearance:menulist only.

This fixes the testcase for me (I haven't tested on Try yet though).

WDYT?

Comment 18

[Louis Chang [:louis]]

Comment on attachment 8939902 [details] [diff] [review]
Render the dropdown button for -moz-appearance:menulist only.

Review of attachment 8939902 [details] [diff] [review]:
-----------------------------------------------------------------

The solution looks good to me. However, I was thinking about whether there is a general solution to fix the appearance issue, which we can't fallback using customized drawing when an appearance value is set and also styled with border or background. But this might be a more complex and big topic. Or maybe we can fix the issue case by case based on the use cases like the solution you provided?

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/36be4f8806ce
part 1 - Render the <select> dropdown button for -moz-appearance:menulist only.  r=louis
https://hg.mozilla.org/integration/mozilla-inbound/rev/981938848b00
part 2 - Reftest.  r=me

Comment 20

[Mats Palmgren (inactive)]

(In reply to Louis Chang [:louis] from comment #18)
> However, I was thinking about whether there
> is a general solution to fix the appearance issue, which we can't fallback
> using customized drawing when an appearance value is set and also styled
> with border or background. But this might be a more complex and big topic.

Yeah, I think we should try to make small incremental changes for now.
Our time is better spent on fixing the blockers for bug 1368555 quickly
so that we can enable -webkit-appearance soon.

Comment 21

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/36be4f8806ce
https://hg.mozilla.org/mozilla-central/rev/981938848b00