Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[css-ui] appearance: Remove 'button-bevel' #3942

Merged
merged 1 commit into from
May 30, 2019

Conversation

zcorpan
Copy link
Member

@zcorpan zcorpan commented May 17, 2019

@MatsPalmgren
Copy link

'listbox' is implemented in Gecko and is different from 'none' (different border size/colors/style and something about the focus ring, maybe also something on Windows which I didn't test but the widget code handles it (see widget/windows/nsNativeThemeWin.cpp in the link), so I think it needs to behave as 'auto'. 'listbox' also appears to be implemented in Safari.

@tkent-google
Copy link
Contributor

I confirmed WebKit had listbox implementation. Chrome recognizes listbox keyword, but does nothing for it.
We should not remove ``listbox``` keyword.

zcorpan added a commit to web-platform-tests/wpt that referenced this pull request May 20, 2019
@zcorpan zcorpan changed the title [css-ui] appearance: Remove 'button-bevel', make 'listbox' alias to 'none' [css-ui] appearance: Remove 'button-bevel' May 20, 2019
@zcorpan
Copy link
Member Author

zcorpan commented May 20, 2019

Thank you @MatsPalmgren and @tkent-google

In my testing, I thought 'listbox' was close enough to 'none' such that it could be an alias. But I've reverted this so that it's an alias to 'auto' again.

@zcorpan zcorpan force-pushed the appearance-removed-keywords branch from edb6305 to 06463de Compare May 21, 2019 15:22
@zcorpan zcorpan requested a review from MatsPalmgren May 21, 2019 18:51
@zcorpan
Copy link
Member Author

zcorpan commented May 24, 2019

Browser bugs:
https://bugzilla.mozilla.org/show_bug.cgi?id=1554150
https://bugs.webkit.org/show_bug.cgi?id=198219

Here's a Live DOM Viewer test to check which values (of those removed in Chromium 75) are supported:
http://software.hixie.ch/utilities/js/live-dom-viewer/saved/6943

@zcorpan zcorpan requested a review from emilio May 29, 2019 13:43
Copy link
Collaborator

@emilio emilio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me. We do have some buttom-bevel rendering support in Gecko, but if we can live without it I'm ok with this.

If for some reason removing it would break something we may have to revisit this.

@frivoal
Copy link
Collaborator

frivoal commented May 30, 2019

Looks good to me. We should be aligning the compat values with the subset of things implemented by browsers that are needed for compat. Values that are being dropped by browsers shouldn't stay in the spec.

Merging.

@zcorpan
Copy link
Member Author

zcorpan commented May 30, 2019

Thank you, @frivoal!

zcorpan added a commit to web-platform-tests/wpt that referenced this pull request Jun 3, 2019
emilio pushed a commit to web-platform-tests/wpt that referenced this pull request Jun 7, 2019
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jun 19, 2019
…rds are supported, a=testonly

Automatic update from web-platform-tests
[css-ui] Update which 'appearance' keywords are supported (#14980)

Follows w3c/csswg-drafts#3942
--

wp5At-commits: 6e850694b1f2e477075f63b02838645b9e1c2b08
wpt-pr: 14980
xeonchen pushed a commit to xeonchen/gecko that referenced this pull request Jun 19, 2019
…rds are supported, a=testonly

Automatic update from web-platform-tests
[css-ui] Update which 'appearance' keywords are supported (#14980)

Follows w3c/csswg-drafts#3942
--

wp5At-commits: 6e850694b1f2e477075f63b02838645b9e1c2b08
wpt-pr: 14980
marcoscaceres pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 23, 2019
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 4, 2019
…rds are supported, a=testonly

Automatic update from web-platform-tests
[css-ui] Update which 'appearance' keywords are supported (#14980)

Follows w3c/csswg-drafts#3942
--

wp5At-commits: 6e850694b1f2e477075f63b02838645b9e1c2b08
wpt-pr: 14980

UltraBlame original commit: 6f99efd9adf7ac916ec5dd0007ae9b9190363cea
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 4, 2019
…rds are supported, a=testonly

Automatic update from web-platform-tests
[css-ui] Update which 'appearance' keywords are supported (#14980)

Follows w3c/csswg-drafts#3942
--

wp5At-commits: 6e850694b1f2e477075f63b02838645b9e1c2b08
wpt-pr: 14980

UltraBlame original commit: 6f99efd9adf7ac916ec5dd0007ae9b9190363cea
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 4, 2019
…rds are supported, a=testonly

Automatic update from web-platform-tests
[css-ui] Update which 'appearance' keywords are supported (#14980)

Follows w3c/csswg-drafts#3942
--

wp5At-commits: 6e850694b1f2e477075f63b02838645b9e1c2b08
wpt-pr: 14980

UltraBlame original commit: 6f99efd9adf7ac916ec5dd0007ae9b9190363cea
@frivoal frivoal added this to Done in appearance Dec 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
appearance
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants