Make WordPress Core

Opened 8 years ago

Last modified 3 months ago

#40940 assigned defect (bug)

Twenty Twelve uses invalid linear-gradient() syntax

Reported by: vrubleg's profile vrubleg Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.7.5
Component: Bundled Theme Keywords: has-patch needs-testing
Focuses: Cc:

Description

Twenty Twelve theme uses such construction for gradients:

	background-image: -moz-linear-gradient(top, #f4f4f4, #e6e6e6);
	background-image: -ms-linear-gradient(top, #f4f4f4, #e6e6e6);
	background-image: -webkit-linear-gradient(top, #f4f4f4, #e6e6e6);
	background-image: -o-linear-gradient(top, #f4f4f4, #e6e6e6);
	background-image: linear-gradient(top, #f4f4f4, #e6e6e6);

Notice this obsolete syntax:

linear-gradient(top, #f4f4f4, #e6e6e6)

And compare it to the specification:
https://developer.mozilla.org/en-US/docs/Web/CSS/linear-gradient

It has to use "to" before the direction. So, it has to be:

linear-gradient(to bottom, #f4f4f4, #e6e6e6)

or like this:

linear-gradient(180deg, #f4f4f4, #e6e6e6)

It works now just because browsers parse this property from prefixed linear gradient, and prefixed linear gradient allows to omit "to", and it is actually parsed as "from". It will stop work when support of obsolete prefixed properties will be removed. And we have to replace all "top" to "to bottom" or to "180deg".

Attachments (1)

40940.patch (384 bytes) - added by sabernhardt 3 years ago.
adding button element to basic button style rules

Download all attachments as: .zip

Change History (10)

#1 @vrubleg
8 years ago

Yeah, and maybe it will be better to remove all prefixed versions. All browsers support it for years, even IE10 and IE11 support it. No any reason to keep this obsolete stuff.

Version 0, edited 8 years ago by vrubleg (next)

#2 @vrubleg
8 years ago

Also I figured out that this huge style isn't used at all:

.comments-area article header h4 {
	position: absolute;
	top: 0;
	right: 0;
	padding: 6px 12px;
	padding: 0.428571429rem 0.857142857rem;
	font-size: 12px;
	font-size: 0.857142857rem;
	font-weight: normal;
	color: #fff;
	background-color: #0088d0;
	background-repeat: repeat-x;
	background-image: -moz-linear-gradient(top, #009cee, #0088d0);
	background-image: -ms-linear-gradient(top, #009cee, #0088d0);
	background-image: -webkit-linear-gradient(top, #009cee, #0088d0);
	background-image: -o-linear-gradient(top, #009cee, #0088d0);
	background-image: linear-gradient(top, #009cee, #0088d0);
	border-radius: 3px;
	border: 1px solid #007cbd;
}

So, linear gradients are really used only for buttons (in 3 places). That's it.

Also the author forgot to put "button," here:

.menu-toggle,
input[type="submit"],
input[type="button"],
input[type="reset"],
article.post-password-required input[type=submit],
.bypostauthor cite span {

It has to be just after the ".menu-toggle,", like in the next styles:

.menu-toggle:hover,
.menu-toggle:focus,
button:hover,
input[type="submit"]:hover,
input[type="button"]:hover,
input[type="reset"]:hover,
article.post-password-required input[type=submit]:hover {

And:

.menu-toggle:active,
.menu-toggle.toggled-on,
button:active,
input[type="submit"]:active,
input[type="button"]:active,
input[type="reset"]:active {

And the last. In the first style for buttons this part is useless:

.bypostauthor cite span

It is overwritten completely in further CSS rules which reset all values, remove shadow, etc. So, the same thing could be done without this unnecessary redundancy.

@sabernhardt
3 years ago

adding button element to basic button style rules

#3 @sabernhardt
3 years ago

  • Keywords has-patch added

Thanks for the report, and sorry for the delayed response.

Since opening this ticket, the gradient syntax was corrected in changeset:45124.

Changeset r22899 applied the gradient to button element in its hover/focus/active states, though not to the button element itself. For consideration, 40940.patch adds that. This would alter the styling for at least the search block's submit button and custom buttons (in Custom HTML or Classic blocks, or within a child theme template).

I did not include changes for the following, but of course they still might be worth editing.

  1. Removing vendor prefixes: Yes, these are probably all unnecessary at this point. For comparison, Twenty Thirteen only has the -webkit prefix, and Twenty Seventeen includes others again. Twenty Twenty and Twenty Twenty-One do not include prefixes for gradients.
  2. Removing the .comments-area article header h4 style: I cannot find an h4 heading in the comments section, even in the original functions.php file.
  3. Editing .bypostauthor cite span: I think this part would be better to leave as it is. The second set overrides almost all properties from the first, but it maintains the font-size and line-height. (Adding those three lines could negate the benefit of removing the three that reset.)

#4 @karmatosed
8 months ago

@sabernhardt considering the way we have been handling gradients in other tickets what are your feelings on still implementing this?

#5 @sabernhardt
8 months ago

  • Keywords 2nd-opinion added

This ticket is not like the ones about adding new gradient styles.

  1. The suggested patch applies existing gradients and other button styles to the button element for more consistency (when sites do not supply different styles). After another two years, I am somewhat more wary of trying to fix that in an old theme, though it could still be worth doing.
  2. The issue in the summary and description was already fixed in #46786.

#6 @karmatosed
8 months ago

  • Keywords dev-feedback added

#7 @karmatosed
8 months ago

  • Keywords 2nd-opinion dev-feedback removed

#8 @karmatosed
5 months ago

  • Keywords needs-testing added
  • Owner set to karmatosed
  • Status changed from new to assigned

Assigning to myself for consideration of approach towards possible commit and testing.

#9 @karmatosed
3 months ago

  • Owner karmatosed deleted
Note: See TracTickets for help on using tickets.