Skip to content

Commit

Permalink
Bug 1700640 - Map width and height to aspect-ratio in <canvas>, <inpu…
Browse files Browse the repository at this point in the history
…t type=image>, and <video>. r=boris

As per https://html.spec.whatwg.org/#attributes-for-embedded-content-and-images:

> The width and height attributes map to the aspect-ratio property on
> img, canvas, and video elements, and input elements with a type
> attribute in the Image Button state.

See whatwg/html#6527 for the parsing issue
with canvas and zero. For now allow both behaviors in the tests.

We also remove the width-and-height-map-to-aspect-ratio pref, as it is
true everywhere and has been for a while.

Differential Revision: https://phabricator.services.mozilla.com/D109618
  • Loading branch information
emilio committed Mar 24, 2021
1 parent c961ad0 commit 7f53643
Show file tree
Hide file tree
Showing 14 changed files with 136 additions and 108 deletions.
20 changes: 20 additions & 0 deletions dom/html/HTMLCanvasElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,26 @@ nsChangeHint HTMLCanvasElement::GetAttributeChangeHint(const nsAtom* aAttribute,
return retval;
}

void HTMLCanvasElement::MapAttributesIntoRule(
const nsMappedAttributes* aAttributes, MappedDeclarations& aDecls) {
MapAspectRatioInto(aAttributes, aDecls);
MapCommonAttributesInto(aAttributes, aDecls);
}

nsMapRuleToAttributesFunc HTMLCanvasElement::GetAttributeMappingFunction()
const {
return &MapAttributesIntoRule;
}

NS_IMETHODIMP_(bool)
HTMLCanvasElement::IsAttributeMapped(const nsAtom* aAttribute) const {
static const MappedAttributeEntry attributes[] = {
{nsGkAtoms::width}, {nsGkAtoms::height}, {nullptr}};
static const MappedAttributeEntry* const map[] = {attributes,
sCommonAttributeMap};
return FindAttributeDependence(aAttribute, map);
}

bool HTMLCanvasElement::ParseAttribute(int32_t aNamespaceID, nsAtom* aAttribute,
const nsAString& aValue,
nsIPrincipal* aMaybeScriptedPrincipal,
Expand Down
5 changes: 5 additions & 0 deletions dom/html/HTMLCanvasElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -281,14 +281,19 @@ class HTMLCanvasElement final : public nsGenericHTMLElement,
const nsAString& aValue,
nsIPrincipal* aMaybeScriptedPrincipal,
nsAttrValue& aResult) override;
NS_IMETHOD_(bool) IsAttributeMapped(const nsAtom* aAttribute) const override;
nsChangeHint GetAttributeChangeHint(const nsAtom* aAttribute,
int32_t aModType) const override;
nsMapRuleToAttributesFunc GetAttributeMappingFunction() const override;

virtual nsresult Clone(dom::NodeInfo*, nsINode** aResult) const override;
nsresult CopyInnerTo(HTMLCanvasElement* aDest);

void GetEventTargetParent(EventChainPreVisitor& aVisitor) override;

static void MapAttributesIntoRule(const nsMappedAttributes* aAttributes,
MappedDeclarations&);

/*
* Helpers called by various users of Canvas
*/
Expand Down
4 changes: 2 additions & 2 deletions dom/html/HTMLInputElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5133,8 +5133,8 @@ void HTMLInputElement::ImageInputMapAttributesIntoRule(
aDecls);
nsGenericHTMLFormElementWithState::MapImageMarginAttributeInto(aAttributes,
aDecls);
nsGenericHTMLFormElementWithState::MapImageSizeAttributesInto(aAttributes,
aDecls);
nsGenericHTMLFormElementWithState::MapImageSizeAttributesInto(
aAttributes, aDecls, MapAspectRatio::Yes);
// Images treat align as "float"
nsGenericHTMLFormElementWithState::MapImageAlignAttributeInto(aAttributes,
aDecls);
Expand Down
3 changes: 2 additions & 1 deletion dom/html/HTMLVideoElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,8 @@ bool HTMLVideoElement::ParseAttribute(int32_t aNamespaceID, nsAtom* aAttribute,

void HTMLVideoElement::MapAttributesIntoRule(
const nsMappedAttributes* aAttributes, MappedDeclarations& aDecls) {
nsGenericHTMLElement::MapImageSizeAttributesInto(aAttributes, aDecls);
nsGenericHTMLElement::MapImageSizeAttributesInto(aAttributes, aDecls,
MapAspectRatio::Yes);
nsGenericHTMLElement::MapCommonAttributesInto(aAttributes, aDecls);
}

Expand Down
50 changes: 32 additions & 18 deletions dom/html/nsGenericHTMLElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1335,6 +1335,28 @@ void nsGenericHTMLElement::MapHeightAttributeInto(
}
}

static void DoMapAspectRatio(const nsAttrValue& aWidth,
const nsAttrValue& aHeight,
MappedDeclarations& aDecls) {
Maybe<double> w;
if (aWidth.Type() == nsAttrValue::eInteger) {
w.emplace(aWidth.GetIntegerValue());
} else if (aWidth.Type() == nsAttrValue::eDoubleValue) {
w.emplace(aWidth.GetDoubleValue());
}

Maybe<double> h;
if (aHeight.Type() == nsAttrValue::eInteger) {
h.emplace(aHeight.GetIntegerValue());
} else if (aHeight.Type() == nsAttrValue::eDoubleValue) {
h.emplace(aHeight.GetDoubleValue());
}

if (w && h) {
aDecls.SetAspectRatio(*w, *h);
}
}

void nsGenericHTMLElement::MapImageSizeAttributesInto(
const nsMappedAttributes* aAttributes, MappedDeclarations& aDecls,
MapAspectRatio aMapAspectRatio) {
Expand All @@ -1346,25 +1368,17 @@ void nsGenericHTMLElement::MapImageSizeAttributesInto(
if (height) {
MapDimensionAttributeInto(aDecls, eCSSProperty_height, *height);
}
if (StaticPrefs::layout_css_width_and_height_map_to_aspect_ratio_enabled() &&
aMapAspectRatio == MapAspectRatio::Yes && width && height) {
Maybe<double> w;
if (width->Type() == nsAttrValue::eInteger) {
w.emplace(width->GetIntegerValue());
} else if (width->Type() == nsAttrValue::eDoubleValue) {
w.emplace(width->GetDoubleValue());
}

Maybe<double> h;
if (height->Type() == nsAttrValue::eInteger) {
h.emplace(height->GetIntegerValue());
} else if (height->Type() == nsAttrValue::eDoubleValue) {
h.emplace(height->GetDoubleValue());
}
if (aMapAspectRatio == MapAspectRatio::Yes && width && height) {
DoMapAspectRatio(*width, *height, aDecls);
}
}

if (w && h && *w != 0 && *h != 0) {
aDecls.SetAspectRatio(*w, *h);
}
void nsGenericHTMLElement::MapAspectRatioInto(
const nsMappedAttributes* aAttributes, MappedDeclarations& aDecls) {
auto* width = aAttributes->GetAttr(nsGkAtoms::width);
auto* height = aAttributes->GetAttr(nsGkAtoms::height);
if (width && height) {
DoMapAspectRatio(*width, *height, aDecls);
}
}

Expand Down
11 changes: 11 additions & 0 deletions dom/html/nsGenericHTMLElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,17 @@ class nsGenericHTMLElement : public nsGenericHTMLElementBase {
static void MapImageSizeAttributesInto(const nsMappedAttributes*,
mozilla::MappedDeclarations&,
MapAspectRatio = MapAspectRatio::No);
/**
* Helper to map the width and height attributes into the aspect-ratio
* property.
*
* If you also map the width/height attributes to width/height (as you should
* for any HTML element that isn't <canvas>) then you should use
* MapImageSizeAttributesInto instead, passing MapAspectRatio::Yes instead, as
* that'd be faster.
*/
static void MapAspectRatioInto(const nsMappedAttributes*,
mozilla::MappedDeclarations&);

/**
* Helper to map `width` attribute into a style struct.
Expand Down
7 changes: 0 additions & 7 deletions modules/libpref/init/StaticPrefList.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6985,13 +6985,6 @@
value: true
mirror: always

# Are the width and height attributes on image-like elements mapped to the
# internal-for-now aspect-ratio property?
- name: layout.css.width-and-height-map-to-aspect-ratio.enabled
type: bool
value: true
mirror: always

# Whether :is() and :where() ignore errors inside their selector lists
# internally, rather than failing to parse altogether.
#
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,2 @@
[canvas-aspect-ratio.html]
[Canvas width and height attributes are used as the surface size with contain:size]
expected: FAIL

[Computed style]
expected: FAIL

[Computed style for invalid ratios]
bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1693029
expected:
if release_or_beta: FAIL
prefs: [layout.css.aspect-ratio.enabled:true]
Original file line number Diff line number Diff line change
@@ -1,9 +1,2 @@
[img-aspect-ratio.html]
prefs: [layout.css.width-and-height-map-to-aspect-ratio.enabled:true]
[Computed style]
expected: FAIL

[Computed style for invalid ratios]
bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1693029
expected:
if release_or_beta: FAIL
prefs: [layout.css.aspect-ratio.enabled:true]
Original file line number Diff line number Diff line change
@@ -1,11 +1,2 @@
[video-aspect-ratio.html]
[Video width and height attributes are not used to infer aspect-ratio]
expected: FAIL

[Computed style]
expected: FAIL

[Computed style for invalid ratios]
bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1693029
expected:
if release_or_beta: FAIL
prefs: [layout.css.aspect-ratio.enabled:true]
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<!doctype html>
<title>Canvas width and height attributes are used as the surface size</title>
<title>Canvas width and height attributes are used as the surface size, and also to infer aspect ratio</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="resources/aspect-ratio.js"></script>
Expand Down Expand Up @@ -37,20 +37,22 @@
assert_ratio(canvas, 2.5);
}, "Canvas width and height attributes are used as the surface size");

test(function() {
test_computed_style("10", "20", "auto 10 / 20");
test_computed_style("0", "1", "auto 0 / 1");
test_computed_style("1", "0", "auto 1 / 0");
test_computed_style("0", "0", "auto 0 / 0");
test_computed_style("0.5", "1.5", "auto 0.5 / 1.5");
}, "Computed style");
test_computed_style("10", "20", "auto 10 / 20");
// These are invalid per spec, but see
// https://github.com/whatwg/html/issues/4961
test_computed_style("0", "1", ["auto", "auto 0 / 1"]);
test_computed_style("1", "0", ["auto", "auto 1 / 0"]);
test_computed_style("0", "0", ["auto", "auto 0 / 0"]);

test(function() {
test_computed_style(null, null, "auto");
test_computed_style("10", null, "auto");
test_computed_style(null, "20", "auto");
test_computed_style("xx", "20", "auto");
test_computed_style("10%", "20", "auto");
}, "Computed style for invalid ratios");
// See https://github.com/whatwg/html/issues/4961:
// https://html.spec.whatwg.org/#attr-canvas-width
// https://html.spec.whatwg.org/#rules-for-parsing-non-negative-integers
test_computed_style("0.5", "1.5", ["auto 0 / 1", "auto 0.5 / 1.5"]);
test_computed_style("10%", "20", ["auto", "auto 10 / 20"]);

test_computed_style(null, null, "auto");
test_computed_style("10", null, "auto");
test_computed_style(null, "20", "auto");
test_computed_style("xx", "20", "auto");

</script>
Original file line number Diff line number Diff line change
Expand Up @@ -68,20 +68,20 @@
assert_ratio(images[6], 133/106, "The original aspect ratio of blue.png");
});

test(function() {
test_computed_style("10", "20", "auto 10 / 20");
test_computed_style("0", "1", "auto 0 / 1");
test_computed_style("1", "0", "auto 1 / 0");
test_computed_style("0", "0", "auto 0 / 0");
test_computed_style("0.5", "1.5", "auto 0.5 / 1.5");
}, "Computed style");
test_computed_style("10", "20", "auto 10 / 20");
// These are invalid per spec, but see
// https://github.com/whatwg/html/issues/4961
test_computed_style("0", "1", "auto 0 / 1");
test_computed_style("1", "0", "auto 1 / 0");
test_computed_style("0", "0", "auto 0 / 0");
// https://html.spec.whatwg.org/#map-to-the-aspect-ratio-property
// https://html.spec.whatwg.org/#rules-for-parsing-non-zero-dimension-values
test_computed_style("0.5", "1.5", "auto 0.5 / 1.5");

test(function() {
test_computed_style(null, null, "auto");
test_computed_style("10", null, "auto");
test_computed_style(null, "20", "auto");
test_computed_style("xx", "20", "auto");
test_computed_style("10%", "20", "auto");
}, "Computed style for invalid ratios");
test_computed_style(null, null, "auto");
test_computed_style("10", null, "auto");
test_computed_style(null, "20", "auto");
test_computed_style("xx", "20", "auto");
test_computed_style("10%", "20", "auto");

</script>
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
function test_computed_style_aspect_ratio(tag, attributes, expected) {
var elem = document.createElement(tag);
for (name in attributes) {
let val = attributes[name];
if (val !== null)
elem.setAttribute(name, val);
}
document.body.appendChild(elem);
assert_equals(getComputedStyle(elem).aspectRatio, expected);
test(function() {
var elem = document.createElement(tag);
for (name in attributes) {
let val = attributes[name];
if (val !== null)
elem.setAttribute(name, val);
}
document.body.appendChild(elem);
let aspectRatio = getComputedStyle(elem).aspectRatio;
if (Array.isArray(expected)) {
assert_in_array(aspectRatio, expected);
} else {
assert_equals(aspectRatio, expected);
}
elem.remove();
}, `${tag} with ${JSON.stringify(attributes)}`);
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<!doctype html>
<title>Video width and height attributes are not used to infer aspect-ratio</title>
<title>Video width and height attributes are used to infer aspect-ratio</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/common/media.js"></script>
Expand Down Expand Up @@ -50,20 +50,19 @@
});
}, "aspect ratio for regular video");

test(function() {
test_computed_style("10", "20", "auto 10 / 20");
test_computed_style("0", "1", "auto 0 / 1");
test_computed_style("1", "0", "auto 1 / 0");
test_computed_style("0", "0", "auto 0 / 0");
test_computed_style("0.5", "1.5", "auto 0.5 / 1.5");
}, "Computed style");
test_computed_style("10", "20", "auto 10 / 20");
test_computed_style("0.5", "1.5", "auto 0.5 / 1.5");

test(function() {
test_computed_style(null, null, "auto");
test_computed_style("10", null, "auto");
test_computed_style(null, "20", "auto");
test_computed_style("xx", "20", "auto");
test_computed_style("10%", "20", "auto");
}, "Computed style for invalid ratios");
// These are invalid per spec, but see
// https://github.com/whatwg/html/issues/4961
test_computed_style("0", "1", ["auto", "auto 0 / 1"]);
test_computed_style("1", "0", ["auto", "auto 1 / 0"]);
test_computed_style("0", "0", ["auto", "auto 0 / 0"]);

test_computed_style(null, null, "auto");
test_computed_style("10", null, "auto");
test_computed_style(null, "20", "auto");
test_computed_style("xx", "20", "auto");
test_computed_style("10%", "20", "auto");

</script>

0 comments on commit 7f53643

Please sign in to comment.