Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(315)

Issue 23465021: Revert 157745 "Add support for the object-position CSS property." (Closed)

Created:
7 years, 3 months ago by ajuma
Modified:
7 years, 3 months ago
CC:
blink-reviews
Visibility:
Public.

Description

Revert 157745 "Add support for the object-position CSS property." This is causing a number of debug layout tests to crash with "ASSERTION FAILED: object-position is not whitelisted for animation" in CSSAnimations::isAnimatableProperty. e.g. see http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20%28dbg%29/builds/12296/steps/webkit_tests/logs/stdio > Add support for the object-position CSS property. > > This is hidden behind an "experimental" runtime flag named > "ObjectFitPosition", together with object-fit. > > This is an implementation of object-position as described in > http://www.w3.org/TR/2012/CR-css3-images-20120417/#object-position > > Object-position is used to offset replaced content within its content > box. Painting is always clipped against the content box, regardless of > the "overflow" property. This property is useful together with > object-fit (to achieve a difference between content box size and > replaced content size, so that specifying alignment is interesting), > but can also be used on its own. > > BUG=236333 > > Review URL: https://chromiumcodereview.appspot.com/22839023 TBR=mstensho@opera.com Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=157751

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+22 lines, -871 lines) Patch
M trunk/LayoutTests/animations/resources/animation-test-helpers.js View 1 chunk +0 lines, -13 lines 0 comments Download
D trunk/LayoutTests/fast/css/object-position.html View 1 chunk +0 lines, -49 lines 0 comments Download
D trunk/LayoutTests/fast/css/object-position-expected.html View 1 chunk +0 lines, -53 lines 0 comments Download
D trunk/LayoutTests/fast/css/object-position-svg.html View 1 chunk +0 lines, -50 lines 0 comments Download
D trunk/LayoutTests/fast/css/object-position-svg-expected.html View 1 chunk +0 lines, -53 lines 0 comments Download
D trunk/LayoutTests/fast/css/object-position-with-fit-contain.html View 1 chunk +0 lines, -50 lines 0 comments Download
D trunk/LayoutTests/fast/css/object-position-with-fit-contain-expected.html View 1 chunk +0 lines, -53 lines 0 comments Download
D trunk/LayoutTests/fast/css/object-position-with-fit-cover.html View 1 chunk +0 lines, -50 lines 0 comments Download
D trunk/LayoutTests/fast/css/object-position-with-fit-cover-expected.html View 1 chunk +0 lines, -53 lines 0 comments Download
D trunk/LayoutTests/fast/css/object-position-with-fit-none.html View 1 chunk +0 lines, -50 lines 0 comments Download
D trunk/LayoutTests/fast/css/object-position-with-fit-none-expected.html View 1 chunk +0 lines, -53 lines 0 comments Download
D trunk/LayoutTests/fast/css/parsing-object-position.html View 1 chunk +0 lines, -80 lines 0 comments Download
D trunk/LayoutTests/fast/css/parsing-object-position-expected.txt View 1 chunk +0 lines, -52 lines 0 comments Download
D trunk/LayoutTests/fast/css/resources/circle-small.svg View 1 chunk +0 lines, -6 lines 0 comments Download
D trunk/LayoutTests/transitions/object-position-transition.html View 1 chunk +0 lines, -34 lines 0 comments Download
D trunk/LayoutTests/transitions/object-position-transition-expected.txt View 1 chunk +0 lines, -2 lines 0 comments Download
M trunk/LayoutTests/webexposed/css-properties-as-js-properties-expected.txt View 1 chunk +0 lines, -1 line 0 comments Download
M trunk/Source/core/css/CSSComputedStyleDeclaration.cpp View 3 chunks +1 line, -9 lines 0 comments Download
M trunk/Source/core/css/CSSParser.h View 1 chunk +0 lines, -2 lines 0 comments Download
M trunk/Source/core/css/CSSParser-in.cpp View 3 chunks +2 lines, -18 lines 0 comments Download
M trunk/Source/core/css/CSSProperties.in View 1 chunk +0 lines, -1 line 0 comments Download
M trunk/Source/core/css/CSSProperty.cpp View 1 chunk +0 lines, -1 line 0 comments Download
M trunk/Source/core/css/CSSPropertyNames.in View 1 chunk +0 lines, -1 line 0 comments Download
M trunk/Source/core/css/Pair.h View 1 chunk +12 lines, -29 lines 0 comments Download
M trunk/Source/core/css/resolver/StyleBuilderCustom.h View 1 chunk +0 lines, -1 line 0 comments Download
M trunk/Source/core/css/resolver/StyleBuilderCustom.cpp View 2 chunks +0 lines, -10 lines 0 comments Download
M trunk/Source/core/page/RuntimeCSSEnabled.cpp View 2 chunks +1 line, -5 lines 0 comments Download
M trunk/Source/core/page/UseCounter.cpp View 2 chunks +1 line, -2 lines 0 comments Download
M trunk/Source/core/page/animation/CSSPropertyAnimation.cpp View 2 chunks +0 lines, -7 lines 0 comments Download
D trunk/Source/core/platform/LengthPoint.h View 1 chunk +0 lines, -65 lines 0 comments Download
M trunk/Source/core/rendering/RenderImage.cpp View 1 chunk +0 lines, -3 lines 0 comments Download
M trunk/Source/core/rendering/RenderReplaced.cpp View 2 chunks +5 lines, -5 lines 0 comments Download
M trunk/Source/core/rendering/style/RenderStyle.h View 3 chunks +0 lines, -3 lines 0 comments Download
M trunk/Source/core/rendering/style/RenderStyle.cpp View 1 chunk +0 lines, -1 line 0 comments Download
M trunk/Source/core/rendering/style/StyleRareNonInheritedData.h View 2 chunks +0 lines, -3 lines 0 comments Download
M trunk/Source/core/rendering/style/StyleRareNonInheritedData.cpp View 3 chunks +0 lines, -3 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
ajuma
7 years, 3 months ago (2013-09-13 14:34:33 UTC) #1
ajuma
Committed patchset #1 manually as r157751.
7 years, 3 months ago (2013-09-13 14:35:41 UTC) #2
mstensho (USE GERRIT)
I'm new, and I'm not sure why or how I should review this. Please tell ...
7 years, 3 months ago (2013-09-13 18:11:58 UTC) #3
ajuma
7 years, 3 months ago (2013-09-13 18:31:13 UTC) #4
Message was sent while issue was closed.
On 2013/09/13 18:11:58, Morten Stenshorne wrote:
> I'm new, and I'm not sure why or how I should review this. Please tell me how
to
> proceed. :)

Though codereview asks you to review this, reverts that have already landed with
a TBR aren't formally reviewed in practice.

> Also: I know I have to fix my code and re-apply it, but do I have to create a
> new review, or can I re-use the old one?

There isn't a consensus about whether to re-use the old codereview issue or to
create a new one. Here's a blink-dev discussion about this:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/F0tc2EeJzao/discus...

Powered by Google App Engine
This is Rietveld 408576698