Bug 1837664 - Improve border shorthand serialization. r=boris,devtools-reviewers

Fix some bugs caught by css/cssom/shorthand-values. In particular:

 * Make the shorthand order match the spec.
 * Omit values when we can.

Fix a subtest that wasn't correct. Shorthands can be serialized as long
as !important matches in all components.

Differential Revision: https://phabricator.services.mozilla.com/D180466
This commit is contained in:
Emilio Cobos Álvarez 2023-06-12 10:20:46 +00:00
Родитель e94dd0270e
Коммит 9331a654a0
7 изменённых файлов: 102 добавлений и 133 удалений

Просмотреть файл

@ -330,9 +330,9 @@ exports.CSS_PROPERTIES = {
"-moz-border-end": {
"isInherited": false,
"subproperties": [
"border-inline-end-color",
"border-inline-end-width",
"border-inline-end-style",
"border-inline-end-width"
"border-inline-end-color"
],
"supports": [
"color"
@ -492,9 +492,9 @@ exports.CSS_PROPERTIES = {
"-moz-border-start": {
"isInherited": false,
"subproperties": [
"border-inline-start-color",
"border-inline-start-width",
"border-inline-start-style",
"border-inline-start-width"
"border-inline-start-color"
],
"supports": [
"color"
@ -4047,18 +4047,18 @@ exports.CSS_PROPERTIES = {
"border": {
"isInherited": false,
"subproperties": [
"border-top-color",
"border-top-style",
"border-top-width",
"border-right-color",
"border-right-style",
"border-top-style",
"border-top-color",
"border-right-width",
"border-bottom-color",
"border-bottom-style",
"border-right-style",
"border-right-color",
"border-bottom-width",
"border-left-color",
"border-left-style",
"border-bottom-style",
"border-bottom-color",
"border-left-width",
"border-left-style",
"border-left-color",
"border-image-outset",
"border-image-repeat",
"border-image-slice",
@ -4185,9 +4185,9 @@ exports.CSS_PROPERTIES = {
"border-block-end": {
"isInherited": false,
"subproperties": [
"border-block-end-color",
"border-block-end-width",
"border-block-end-style",
"border-block-end-width"
"border-block-end-color"
],
"supports": [
"color"
@ -4301,9 +4301,9 @@ exports.CSS_PROPERTIES = {
"border-block-start": {
"isInherited": false,
"subproperties": [
"border-block-start-color",
"border-block-start-width",
"border-block-start-style",
"border-block-start-width"
"border-block-start-color"
],
"supports": [
"color"
@ -4460,9 +4460,9 @@ exports.CSS_PROPERTIES = {
"border-bottom": {
"isInherited": false,
"subproperties": [
"border-bottom-color",
"border-bottom-width",
"border-bottom-style",
"border-bottom-width"
"border-bottom-color"
],
"supports": [
"color"
@ -4904,9 +4904,9 @@ exports.CSS_PROPERTIES = {
"border-inline-end": {
"isInherited": false,
"subproperties": [
"border-inline-end-color",
"border-inline-end-width",
"border-inline-end-style",
"border-inline-end-width"
"border-inline-end-color"
],
"supports": [
"color"
@ -5020,9 +5020,9 @@ exports.CSS_PROPERTIES = {
"border-inline-start": {
"isInherited": false,
"subproperties": [
"border-inline-start-color",
"border-inline-start-width",
"border-inline-start-style",
"border-inline-start-width"
"border-inline-start-color"
],
"supports": [
"color"
@ -5179,9 +5179,9 @@ exports.CSS_PROPERTIES = {
"border-left": {
"isInherited": false,
"subproperties": [
"border-left-color",
"border-left-width",
"border-left-style",
"border-left-width"
"border-left-color"
],
"supports": [
"color"
@ -5312,9 +5312,9 @@ exports.CSS_PROPERTIES = {
"border-right": {
"isInherited": false,
"subproperties": [
"border-right-color",
"border-right-width",
"border-right-style",
"border-right-width"
"border-right-color"
],
"supports": [
"color"
@ -5497,9 +5497,9 @@ exports.CSS_PROPERTIES = {
"border-top": {
"isInherited": false,
"subproperties": [
"border-top-color",
"border-top-width",
"border-top-style",
"border-top-width"
"border-top-color"
],
"supports": [
"color"

Просмотреть файл

@ -45,25 +45,12 @@ is(e.style.border, "", "should not be able to serialize border");
// Test suppression of currentcolor in border shorthands.
e.setAttribute("style", "border: medium solid");
ok(e.style.border == "medium solid" ||
e.style.border == "solid medium",
"implied default color omitted serializing border");
ok(e.style.borderLeft == "medium solid" ||
e.style.borderLeft == "solid medium",
"implied default color omitted serializing border-left");
ok(e.style.cssText == "border: medium solid;" ||
e.style.cssText == "border: solid medium;",
"implied default color omitted serializing declaration");
is(e.style.border, "solid", "implied default color omitted serializing border");
is(e.style.borderLeft, "solid", "implied default color omitted serializing border-left");
is(e.style.cssText, "border: solid;", "implied default color omitted serializing declaration");
e.setAttribute("style", "border-right: medium solid");
ok(e.style.borderRight == "medium solid" ||
e.style.borderRight == "solid medium",
"implied default color omitted serializing border-right");
ok(e.style.borderRight == "medium solid" ||
e.style.borderRight == "solid medium",
"implied default color omitted serializing border-right");
ok(e.style.cssText == "border-right: medium solid;" ||
e.style.cssText == "border-right: solid medium;",
"implied default color omitted serializing declaration");
is(e.style.borderRight, "solid", "implied default color omitted serializing border-right");
is(e.style.cssText, "border-right: solid;", "implied default color omitted serializing declaration");
// Test that we shorten box properties to the shortest possible.
e.setAttribute("style", "margin: 7px");
@ -239,12 +226,10 @@ is(e.style.animation, "", "should not have animation shorthand (lists different
// (bug 482692).
e.setAttribute("style", 'border-image: url("foo.png") 5 5 5 5 / 5 5 5 5 / 5 5 5 5 repeat repeat; border-left: medium solid green');
is(e.style.cssText,
'border-image: url("foo.png") 5 / 5 / 5 repeat; border-left: medium solid green;',
'border-image: url("foo.png") 5 / 5 / 5 repeat; border-left: solid green;',
"border-left does NOT reset border-image");
e.setAttribute("style", 'border-image: url("foo.png") 5 5 5 5; border: medium solid green');
is(e.style.cssText,
'border: medium solid green;',
"border DOES reset border-image");
e.setAttribute("style", 'border-image: url("foo.png") 5 5 5 5; border: solid green');
is(e.style.cssText, 'border: solid green;', "border DOES reset border-image");
// Test that the color goes at the beginning of the last item of the
// background shorthand.

Просмотреть файл

@ -106,32 +106,6 @@ pub mod shorthands {
use style_traits::{ParseError, StyleParseErrorKind};
use crate::values::specified;
use style_traits::{CssWriter, ToCss};
use crate::values::specified::{BorderStyle, Color};
use std::fmt::{self, Write};
fn serialize_directional_border<W, I,>(
dest: &mut CssWriter<W>,
width: &I,
style: &BorderStyle,
color: &Color,
) -> fmt::Result
where
W: Write,
I: ToCss,
{
width.to_css(dest)?;
// FIXME(emilio): Should we really serialize the border style if it's
// `solid`?
dest.write_char(' ')?;
style.to_css(dest)?;
if *color != Color::CurrentColor {
dest.write_char(' ')?;
color.to_css(dest)?;
}
Ok(())
}
% for style_struct in data.style_structs:
include!("${repr(os.path.join(OUT_DIR, 'shorthands/{}.rs'.format(style_struct.name_lower)))[1:-1]}");
% endfor

Просмотреть файл

@ -107,7 +107,7 @@ pub fn parse_border<'i, 't>(
engines="gecko servo-2013 servo-2020"
sub_properties="${' '.join(
'border-%s-%s' % (side, prop)
for prop in ['color', 'style', 'width']
for prop in ['width', 'style', 'color']
)}"
aliases="${maybe_moz_logical_alias(engine, (side, logical), '-moz-border-%s')}"
spec="${spec}">
@ -126,7 +126,7 @@ pub fn parse_border<'i, 't>(
impl<'a> ToCss for LonghandsToSerialize<'a> {
fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
super::serialize_directional_border(
crate::values::specified::border::serialize_directional_border(
dest,
self.border_${to_rust_ident(side)}_width,
self.border_${to_rust_ident(side)}_style,
@ -141,8 +141,8 @@ pub fn parse_border<'i, 't>(
<%helpers:shorthand name="border"
engines="gecko servo-2013 servo-2020"
sub_properties="${' '.join('border-%s-%s' % (side, prop)
for side in PHYSICAL_SIDES
for prop in ['color', 'style', 'width'])}
for side in PHYSICAL_SIDES for prop in ['width', 'style', 'color']
)}
${' '.join('border-image-%s' % name
for name in ['outset', 'repeat', 'slice', 'source', 'width'])}"
derive_value_info="False"
@ -205,16 +205,15 @@ pub fn parse_border<'i, 't>(
// If all longhands are all present, then all sides should be the same,
// so we can just one set of color/style/width
if all_equal {
super::serialize_directional_border(
if !all_equal {
return Ok(())
}
crate::values::specified::border::serialize_directional_border(
dest,
self.border_${side}_width,
self.border_${side}_style,
self.border_${side}_color
)
} else {
Ok(())
}
}
}
@ -358,14 +357,27 @@ pub fn parse_border<'i, 't>(
impl<'a> ToCss for LonghandsToSerialize<'a> {
fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
self.border_image_source.to_css(dest)?;
% for name in "slice outset width repeat".split():
let has_${name} = *self.border_image_${name} != border_image_${name}::get_initial_specified_value();
% endfor
let needs_slice = has_slice || has_width || has_outset;
if needs_slice {
dest.write_char(' ')?;
self.border_image_slice.to_css(dest)?;
if has_width {
dest.write_str(" / ")?;
self.border_image_width.to_css(dest)?;
}
if has_outset {
dest.write_str(" / ")?;
self.border_image_outset.to_css(dest)?;
}
}
if has_repeat {
dest.write_char(' ')?;
self.border_image_repeat.to_css(dest)
self.border_image_repeat.to_css(dest)?;
}
Ok(())
}
}
</%helpers:shorthand>
@ -453,7 +465,7 @@ pub fn parse_border<'i, 't>(
impl<'a> ToCss for LonghandsToSerialize<'a> {
fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
super::serialize_directional_border(
crate::values::specified::border::serialize_directional_border(
dest,
self.border_${axis}_start_width,
self.border_${axis}_start_style,

Просмотреть файл

@ -16,10 +16,11 @@ use crate::values::generics::rect::Rect;
use crate::values::generics::size::Size2D;
use crate::values::specified::length::{NonNegativeLength, NonNegativeLengthPercentage};
use crate::values::specified::{AllowQuirks, NonNegativeNumber, NonNegativeNumberOrPercentage};
use crate::values::specified::Color;
use crate::Zero;
use cssparser::Parser;
use std::fmt::{self, Write};
use style_traits::{CssWriter, ParseError, ToCss};
use style_traits::{CssWriter, ParseError, ToCss, values::SequenceWriter};
/// A specified value for a single side of a `border-style` property.
///
@ -316,3 +317,32 @@ impl Parse for BorderImageRepeat {
))
}
}
/// Serializes a border shorthand value composed of width/style/color.
pub fn serialize_directional_border<W>(
dest: &mut CssWriter<W>,
width: &BorderSideWidth,
style: &BorderStyle,
color: &Color,
) -> fmt::Result
where
W: Write,
{
let has_style = *style != BorderStyle::None;
let has_color = *color != Color::CurrentColor;
let has_width = *width != BorderSideWidth::Medium;
if !has_style && !has_color && !has_width {
return width.to_css(dest)
}
let mut writer = SequenceWriter::new(dest, " ");
if has_width {
writer.item(width)?;
}
if has_style {
writer.item(style)?;
}
if has_color {
writer.item(color)?;
}
Ok(())
}

Просмотреть файл

@ -1,32 +0,0 @@
[shorthand-values.html]
expected:
if (os == "android") and fission: [OK, TIMEOUT]
[The serialization of border: 1px; border-top: 1px; should be canonical.]
expected: FAIL
[The serialization of border: 1px red; should be canonical.]
expected: FAIL
[The serialization of border: red; should be canonical.]
expected: FAIL
[The serialization of border-top: 1px; border-right: 1px; border-bottom: 1px; border-left: 1px; should be canonical.]
expected: FAIL
[The serialization of border-top: 1px; border-right: 2px; border-bottom: 3px; border-left: 4px; should be canonical.]
expected: FAIL
[The serialization of border: 1px; border-top: 2px; should be canonical.]
expected: FAIL
[The serialization of border: 1px; border-top: 1px !important; should be canonical.]
expected: FAIL
[The serialization of border: 1px; border-top-color: red; should be canonical.]
expected: FAIL
[The serialization of border: solid; border-style: dotted should be canonical.]
expected: FAIL
[The serialization of border-top: 1px; border-right: 1px; border-bottom: 1px; border-left: 1px; border-image: none; should be canonical.]
expected: FAIL

Просмотреть файл

@ -29,7 +29,7 @@
'border-top: 1px; border-right: 1px; border-bottom: 1px; border-left: 1px;': 'border-width: 1px; border-style: none; border-color: currentcolor;',
'border-top: 1px; border-right: 2px; border-bottom: 3px; border-left: 4px;': 'border-width: 1px 2px 3px 4px; border-style: none; border-color: currentcolor;',
'border: 1px; border-top: 2px;': 'border-width: 2px 1px 1px; border-style: none; border-color: currentcolor; border-image: none;',
'border: 1px; border-top: 1px !important;': 'border-right-width: 1px; border-bottom-width: 1px; border-left-width: 1px; border-right-style: none; border-bottom-style: none; border-left-style: none; border-right-color: currentcolor; border-bottom-color: currentcolor; border-left-color: currentcolor; border-image: none; border-top-width: 1px !important; border-top-style: none !important; border-top-color: currentcolor !important;',
'border: 1px; border-top: 1px !important;': 'border-right: 1px; border-bottom: 1px; border-left: 1px; border-image: none; border-top: 1px !important;',
'border: 1px; border-top-color: red;': 'border-width: 1px; border-style: none; border-color: red currentcolor currentcolor; border-image: none;',
'border: solid; border-style: dotted': 'border: dotted;',
'border-width: 1px;': 'border-width: 1px;',