Make WordPress Core

Opened 8 years ago

Closed 21 months ago

Last modified 21 months ago

#38431 closed defect (bug) (fixed)

-webkit-appearance rule should be removed from the CSS

Reported by: laurent22777's profile laurent22777 Owned by: audrasjb's profile audrasjb
Milestone: 6.1 Priority: normal
Severity: normal Version: 4.3
Component: General Keywords: good-first-bug commit has-patch
Focuses: css Cc:

Description

The -webkit-appearance rule used in customize-control.css is not supported properly on all browsers and in some cases might make certain layouts impossible. Please see the MDN article about it:

https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-appearance

As mentioned there, even the "none" value will have different behavior on different browsers. In my case, it forces the element to wrap to the next line on Chrome, and the only way to go around this is to delete the rule directly in the WordPress CSS.

Change History (18)

#1 @desrosj
5 years ago

  • Keywords reporter-feedback added
  • Version changed from 4.6.1 to 4.3

Hi @laurent22777!

Welcome to Trac! My apologies that it took so long to receive a response.

Are you still experiencing this issue? Can you provide some screenshots and code to help others reproduce the issue?

Ticket this was introduced in: #31336

#2 @desrosj
5 years ago

  • Component changed from General to Customize
  • Focuses ui removed

#3 @laurent22777
5 years ago

Well unless you lend me your time machine I can't provide screenshots of something I was working on three years ago :-)

All I know is that for sure the rule "-webkit-appearance" should be removed from the CSS because it doesn't work well, and once it's set it cannot be overridden by the user. As the post said, my fix was to remove it directly from the WordPress CSS.

#4 @celloexpressions
3 years ago

  • Focuses css added
  • Keywords needs-patch 2nd-opinion added; reporter-feedback removed

Does anyone regularly working with core CSS have input on the use of webkit-appearance? Can/should it be removed from any uses in core?

#5 @desrosj
3 years ago

  • Keywords close added; needs-patch removed

Given the data shown on Can I Use, I'm inclined to say these should not be removed. On this page, it's noted that all versions of Safari on desktop and on iOS require the -webkit- prefix for the appearance property to allow for partial support.

Edge, Chrome (-webkit- prefix), and Firefox (-moz- prefix) are no longer required for the appearance property in the versions supported by today's versions of WordPress.

The linked developer documentation above now includes the following clarification:

The differences are smaller in the newest browsers.

Browsers seem to have made the behavior of this property more consistent. So I'm not sure this is really an issue worth addressing anymore. I'm suggesting this one is a close, but would still like someone with more experience using the property to weigh in.

#6 @desrosj
3 years ago

  • Component changed from Customize to General

I'm also going to move this to the General component. The property is used throughout Core, and a coordinated effort would be needed if it should be removed from Core's CSS.

#7 @desrosj
22 months ago

  • Keywords needs-patch good-first-bug added; 2nd-opinion close removed
  • Milestone changed from Awaiting Review to 6.1

Came back to this one going through tickets with a close suggestion and took another look.

The -webkit- prefix no longer seems to be required for any of the browser versions currently supported by Core. After some testing, it seems that the prefix is simply ignored and the property is interpreted as appearance.

There are a fair number of occurrences in bundled themes. I don't feel strongly about keeping these, but there could be other considerations to be had in themes vs. Core admin CSS. We also will not be able to change the occurrences found in third party libraries included within the code base (tinymce and medialement to name a few).

@peterwilsoncc Can you think of any reason why this should not be changed?

This ticket was mentioned in Slack in #core by chaion07. View the logs.


21 months ago

#9 @costdev
21 months ago

  • Keywords needs-testing dev-feedback added

This ticket was reviewed in the recent bug scrub. The issue still needs to be verified/reproduced and there is a request for feedback on the implications of removing -webkit-appearance.

Adding needs-testing and dev-feedback.

Additional props: @hilayt24, @chaion07

This ticket was mentioned in PR #3240 on WordPress/wordpress-develop by rolf-yoast.


21 months ago
#10

  • Keywords has-patch added; needs-patch removed

Removes unneeded CSS from the customize-control.css

Trac ticket: https://core.trac.wordpress.org/ticket/38431

#11 @rolfsiebers
21 months ago

  • Keywords needs-patch added; has-patch removed

I've removed the -webkit-appearance: none in Chrome but it didn't make any difference for me. So as far as I can see it can be removed without any consequences.

@igorsch helped me with testing.

We've also made a PR to remove this CSS line.

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


21 months ago

#13 @audrasjb
21 months ago

  • Keywords commit added; needs-testing removed
  • Owner set to audrasjb
  • Status changed from new to accepted

As per today's bug scrub: we reviewed this with @Clorith and it looks good to go.
I also tested the PR and removing this CSS declaration doesn't change anything.

Self assigning for commit.

#14 @audrasjb
21 months ago

  • Keywords has-patch added; needs-patch dev-feedback removed

#15 @audrasjb
21 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 54188:

Code Modernization: Remove -webkit-appearance CSS declarations.

This changeset removes the deprecated -webkit-appearance CSS property used in customizer-controls.css. It doesn't change anything to the interface.

Props laurent22777, desrosj, celloexpressions, costdev, rolfsiebers, audrasjb, Clorith.
Fixes #38431.

#17 @SergeyBiryukov
21 months ago

Thanks for the commit!

Note: I've updated the props list in make/core admin to add @igorsch for help with testing, per comment:11.

#18 @audrasjb
21 months ago

Ah thank you Sergey, I completely missed that \o/

Note: See TracTickets for help on using tickets.