Chromium Code Reviews
Help | Chromium Project | Sign in
(30)

Issue 1709963002: [css-align] New CSS Value 'normal' for Self Alignment (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 1 month ago by jfernandez
Modified:
7 months, 2 weeks ago
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, blink-reviews-dom_chromium.org, blink-reviews-layout_chromium.org, blink-reviews-style_chromium.org, chromium-reviews, dglazkov+blink, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, pdr+renderingwatchlist_chromium.org, Manuel Rego, rwlbuis, sof, svillar, szager+layoutwatch_chromium.org, zoltan1, Eddy
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[css-align] New CSS Value 'normal' for Self Alignment The Box Alignment specification defines a new value 'normal' to be used as default for the different layout models, which will define the specific behavior for each case. This patch adds a new CSS value in the parsing logic and adapts the Self Alignment properties to the new value. The 'auto' value is no longer valid for the 'align-items' property and the Computed Value will be always the specified value. However, it's still valid for justify-items and both of Self Alignment properties. We will use the StyleAdjuster to resolve these 'auto' values. In order to deal with repaint and relayout whenever some alignment property changes and resolve again the 'auto' values, current implementation relies on triggering a Reattach whenever any of these properties changes. This is a source of many problems for animations, plugins, ... (See bug 580070 for details). The correct way of implementing this is using "Inherit" instead so we can run the StyleAdjuster again only for the nodes affected by the change. This approach reduces considerably the number of style invalidations and let us unskip the align-items repaint and relayout related tests. These, as well as the justify-items related ones, had to be rebaselined due the different invalidation areas generated. Because of resolving everything in the StyleAdjuster, we don't need to duplicate the resolution logic for generating the computed style. We only have to deal with the 'auto' flag duality, which is not both 'auto' and 'normal' in any post-adjustement logic. This also gives us the opportunity to get rid of the ShadowDOM and slotted elements issues, since we don't have to deal with parent's style. Additionally, this patch updates the layout logic as well, for both Flexbox and Grid layout models. BUG=565883, 474798, 448371, 582230 Committed: https://crrev.com/fe59cb90e7b08f1faa146088ddbd7589d87eebb5 Cr-Commit-Position: refs/heads/master@{#412728}

Patch Set 1 #

Total comments: 8

Patch Set 2 : Fixed layout tests. #

Patch Set 3 : Skipped the correct test. #

Patch Set 4 : Don't implement Document::virtualEnsureComputedStyle. #

Patch Set 5 : Minor changes. #

Total comments: 4

Patch Set 6 : Removed the Reattach issue sutff. #

Patch Set 7 : Applied sugested changes in the layout tests. #

Patch Set 8 : Patch rebased. #

Total comments: 1

Patch Set 9 : Applied suggested changes. #

Total comments: 2

Patch Set 10 : Added a new test for DOM and ShadowDOM root elements. #

Patch Set 11 : Added some additional test cases for 'slot' elements. #

Patch Set 12 : New approach: Using the style inheritance mechanism. #

Patch Set 13 : Final agreed approach based on the StyleAdjuster. #

Patch Set 14 : Changes to reduce the copy-on-write impact. #

Patch Set 15 : Unskipped some repaint tests and rebaselined. #

Total comments: 9

Patch Set 16 : Patch rebased. #

Patch Set 17 : Applied suggested changes. #

Total comments: 1

Patch Set 18 : Hack to allow StyleAdjuster considering Anonymous box's style as parentStyle. #

Patch Set 19 : Fixed repaint tests. #

Total comments: 2

Patch Set 20 : Patch rebased and removed the StyleAdjuster hack. #

Patch Set 21 : Getting back the logic for 'auto' resolution at layout time. #

Patch Set 22 : Fixed the issue with FullScreen without needing to hack flexbox's auto resolution logic. #

Patch Set 23 : New test for alignment and anonymous boxes. #

Total comments: 8

Patch Set 24 : Patch for landing. #

Patch Set 25 : Applied suggested changed and using testharness instead of js-test. #

Patch Set 26 : Fixed layout tests. #

Patch Set 27 : Patch rebased. Getting back the Flexbox styleDidChange logic. #

Patch Set 28 : Temp change to see how it behaves in mac bots. #

Patch Set 29 : A different approach to solve the forms related failures. #

Patch Set 30 : Patch rebased. Going further on the new approach. #

Patch Set 31 : A different approach to solve the forms related failures. #

Patch Set 32 : Patch rebased for landing. #

Patch Set 33 : Getting back the FullScreen fix, missed during the rebase. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+944 lines, -657 lines) Patch
M third_party/WebKit/LayoutTests/TestExpectations View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 2 chunks +0 lines, -4 lines 0 comments Download
M third_party/WebKit/LayoutTests/css3/flexbox/css-properties.html View 3 chunks +18 lines, -17 lines 0 comments Download
M third_party/WebKit/LayoutTests/css3/flexbox/css-properties-expected.txt View 3 chunks +16 lines, -16 lines 0 comments Download
M third_party/WebKit/LayoutTests/csspaint/style-background-image.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/csspaint/style-background-image-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 2 chunks +2 lines, -2 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/alignment/alignment-and-anomymous-boxes.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +25 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/alignment/alignment-and-anomymous-boxes-expected.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +25 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/alignment/ensure-flexbox-compatibility-with-initial-values-expected.txt View 1 chunk +8 lines, -8 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/alignment/parse-align-items.html View 1 2 3 4 5 6 4 chunks +10 lines, -7 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/alignment/parse-align-items-expected.txt View 1 2 3 4 5 6 2 chunks +36 lines, -34 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/alignment/parse-align-self.html View 1 2 3 4 5 6 7 8 9 3 chunks +15 lines, -10 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/alignment/parse-align-self-expected.txt View 1 2 3 4 5 6 7 8 2 chunks +38 lines, -34 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/alignment/parse-alignment-of-root-elements.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 1 chunk +279 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/alignment/parse-justify-items.html View 1 2 3 4 5 6 7 8 9 3 chunks +12 lines, -7 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/alignment/parse-justify-items-expected.txt View 1 2 3 4 5 6 7 8 2 chunks +35 lines, -31 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/alignment/parse-justify-self.html View 1 2 3 4 5 6 7 8 9 3 chunks +15 lines, -10 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/alignment/parse-justify-self-expected.txt View 1 2 3 4 5 6 7 8 2 chunks +38 lines, -34 lines 0 comments Download
A + third_party/WebKit/LayoutTests/fast/alignment/resources/alignment-parsing-utils-th.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/css/getComputedStyle/computed-style-listing-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 2 chunks +4 lines, -4 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/css/getComputedStyle/computed-style-without-renderer-listing-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 2 chunks +4 lines, -4 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/repaint/align-items-change.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/repaint/align-items-change-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 1 chunk +33 lines, -23 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/repaint/align-items-overflow-change.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/repaint/align-items-overflow-change-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 1 chunk +32 lines, -26 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/repaint/align-self-change-keeping-geometry-grid.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 2 chunks +14 lines, -7 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/repaint/align-self-change-keeping-geometry-grid-expected.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 1 chunk +13 lines, -6 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/repaint/align-self-change-keeping-geometry-grid-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/repaint/justify-items-change-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 1 chunk +6 lines, -33 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/repaint/justify-items-legacy-change-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 1 chunk +8 lines, -80 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/repaint/justify-items-overflow-change-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 1 chunk +6 lines, -69 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/repaint/justify-self-change-keeping-geometry.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 3 chunks +9 lines, -13 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/repaint/justify-self-change-keeping-geometry-expected.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 1 chunk +9 lines, -12 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/repaint/justify-self-change-keeping-geometry-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/svg/css/getComputedStyle-listing-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 2 chunks +4 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 2 chunks +8 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/css/CSSProperties.in View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 3 chunks +12 lines, -31 lines 0 comments Download
M third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 3 chunks +12 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/css/resolver/StyleAdjuster.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 1 chunk +15 lines, -22 lines 0 comments Download
M third_party/WebKit/Source/core/html/shadow/TextControlInnerElements.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 2 chunks +6 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutBox.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 2 chunks +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutBox.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 4 chunks +13 lines, -12 lines 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutButton.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutFlexibleBox.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 9 chunks +16 lines, -12 lines 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutFullScreen.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 30 31 32 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutFullScreen.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutGrid.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutGrid.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 11 chunks +32 lines, -11 lines 0 comments Download
M third_party/WebKit/Source/core/style/ComputedStyle.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 5 chunks +53 lines, -41 lines 0 comments Download
M third_party/WebKit/Source/core/style/ComputedStyle.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 2 chunks +39 lines, -20 lines 0 comments Download
M third_party/WebKit/Source/core/style/ComputedStyleConstants.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 1 chunk +2 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/style/StyleRareNonInheritedData.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 1 chunk +1 line, -1 line 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 120 (34 generated)
jfernandez
1 year, 1 month ago (2016-02-18 11:33:41 UTC) #2
cbiesinger
lgtm for flexbox/grid https://codereview.chromium.org/1709963002/diff/1/third_party/WebKit/LayoutTests/TestExpectations File third_party/WebKit/LayoutTests/TestExpectations (right): https://codereview.chromium.org/1709963002/diff/1/third_party/WebKit/LayoutTests/TestExpectations#newcode1042 third_party/WebKit/LayoutTests/TestExpectations:1042: crbug.com/587641 fast/repaint/justify-items-change.html [ Failure ] Why ...
1 year, 1 month ago (2016-02-18 20:13:17 UTC) #3
jfernandez
https://codereview.chromium.org/1709963002/diff/1/third_party/WebKit/LayoutTests/TestExpectations File third_party/WebKit/LayoutTests/TestExpectations (right): https://codereview.chromium.org/1709963002/diff/1/third_party/WebKit/LayoutTests/TestExpectations#newcode1042 third_party/WebKit/LayoutTests/TestExpectations:1042: crbug.com/587641 fast/repaint/justify-items-change.html [ Failure ] On 2016/02/18 20:13:17, cbiesinger ...
1 year, 1 month ago (2016-02-18 21:28:58 UTC) #4
jfernandez
1 year, 1 month ago (2016-02-23 15:53:35 UTC) #5
Timothy Loh
https://codereview.chromium.org/1709963002/diff/1/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp File third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp (right): https://codereview.chromium.org/1709963002/diff/1/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp#newcode396 third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp:396: if (parent) On 2016/02/18 21:28:57, jfernandez wrote: > On ...
1 year, 1 month ago (2016-02-25 00:23:17 UTC) #6
jfernandez
https://codereview.chromium.org/1709963002/diff/1/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp File third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp (right): https://codereview.chromium.org/1709963002/diff/1/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp#newcode396 third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp:396: if (parent) On 2016/02/25 00:23:17, Timothy Loh wrote: > ...
1 year, 1 month ago (2016-02-25 01:52:37 UTC) #7
jfernandez
Patch Set 4 introduces some important changes in the approach. As @timloh commented, we may ...
1 year ago (2016-03-01 20:36:38 UTC) #8
jfernandez
1 year ago (2016-03-04 22:43:05 UTC) #11
Timothy Loh
Can we split this up into a change for adding normal/removing auto and a change ...
1 year ago (2016-03-07 05:58:32 UTC) #12
jfernandez
https://codereview.chromium.org/1709963002/diff/80001/third_party/WebKit/LayoutTests/fast/alignment/parse-align-items.html File third_party/WebKit/LayoutTests/fast/alignment/parse-align-items.html (right): https://codereview.chromium.org/1709963002/diff/80001/third_party/WebKit/LayoutTests/fast/alignment/parse-align-items.html#newcode203 third_party/WebKit/LayoutTests/fast/alignment/parse-align-items.html:203: <!-- The 'auto' value is no valid for the ...
1 year ago (2016-03-14 09:29:29 UTC) #13
Timothy Loh
Patch description needs to be updated. Is this fixing our behaviour in shadow dom? If ...
1 year ago (2016-03-15 00:30:28 UTC) #15
jfernandez
Adding @hayato to the loop since I'm not totally sure my patch addresses all the ...
1 year ago (2016-03-16 22:25:41 UTC) #17
kochi
https://codereview.chromium.org/1709963002/diff/180001/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp File third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp (right): https://codereview.chromium.org/1709963002/diff/180001/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp#newcode382 third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp:382: if (isTreeScopeRoot(parent)) As FlatTreeTraversal::parent() returns shadow host instead of ...
1 year ago (2016-03-18 05:19:03 UTC) #19
hayato
On 2016/03/16 at 22:25:41, jfernandez wrote: > Adding @hayato to the loop since I'm not ...
1 year ago (2016-03-18 07:14:07 UTC) #20
jfernandez
On 2016/03/18 07:14:07, hayato wrote: > On 2016/03/16 at 22:25:41, jfernandez wrote: > > Adding ...
1 year ago (2016-03-18 13:54:41 UTC) #21
jfernandez
https://codereview.chromium.org/1709963002/diff/180001/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp File third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp (right): https://codereview.chromium.org/1709963002/diff/180001/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp#newcode382 third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp:382: if (isTreeScopeRoot(parent)) On 2016/03/18 05:19:03, kochi wrote: > As ...
1 year ago (2016-03-18 13:54:54 UTC) #22
jfernandez
On 2016/03/18 13:54:54, jfernandez wrote: > https://codereview.chromium.org/1709963002/diff/180001/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp > File third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp > (right): > > https://codereview.chromium.org/1709963002/diff/180001/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp#newcode382 ...
1 year ago (2016-03-23 15:08:13 UTC) #24
jfernandez
I've added a new layout tests to check out how the root elements, both in ...
1 year ago (2016-03-23 15:22:37 UTC) #25
esprehn
I've been thinking about this recently and I think we might want to take a ...
1 year ago (2016-03-23 17:49:59 UTC) #26
esprehn
rune@ what do you think?
1 year ago (2016-03-23 17:53:58 UTC) #27
rune
On 2016/03/23 17:53:58, esprehn wrote: > rune@ what do you think? sgtm I'm on a ...
1 year ago (2016-03-23 22:10:34 UTC) #28
rune
On 2016/03/23 22:10:34, rune (Easter - slow) wrote: > On 2016/03/23 17:53:58, esprehn wrote: > ...
1 year ago (2016-03-24 09:49:45 UTC) #29
jfernandez
Thanks @esprhen and @rune for the feedback. I like the idea of using inheritance to ...
1 year ago (2016-03-28 11:16:22 UTC) #30
jfernandez
Oh, for got to mention, not sure if you all ware aware that this "inheritance" ...
1 year ago (2016-03-28 11:50:22 UTC) #31
jfernandez
Another issue to address with the StyleAdjuster + inheritance is that changing the value of ...
1 year ago (2016-03-28 12:29:54 UTC) #32
Timothy Loh
Using StyleAdjuster sgtm. I don't think we need special values on the inherited data structures ...
1 year ago (2016-03-29 04:40:02 UTC) #33
jfernandez
On 2016/03/29 04:40:02, Timothy Loh wrote: > Using StyleAdjuster sgtm. I don't think we need ...
1 year ago (2016-03-29 08:04:05 UTC) #34
Timothy Loh
On 2016/03/29 08:04:05, jfernandez wrote: > The StyleAdjuster is not run after parent's align-items changes ...
1 year ago (2016-03-30 00:32:19 UTC) #35
jfernandez
On 2016/03/30 00:32:19, Timothy Loh wrote: > On 2016/03/29 08:04:05, jfernandez wrote: > > The ...
12 months ago (2016-03-30 07:55:26 UTC) #36
jfernandez
Just realized that the approach suggested by @esprehn and @rune implies that stylePropagationDiff will returning ...
12 months ago (2016-03-30 09:07:04 UTC) #37
jfernandez
I'm having problems implementing @rune's idea. I've found out that StyleResolver manages a styles cache ...
12 months ago (2016-03-30 14:35:30 UTC) #38
jfernandez
I finally managed to do a preliminary implementation of the approach proposed by @esprehn and ...
12 months ago (2016-03-30 21:40:25 UTC) #39
jfernandez
On 2016/03/30 00:32:19, Timothy Loh wrote: > On 2016/03/29 08:04:05, jfernandez wrote: > > The ...
12 months ago (2016-04-01 13:46:50 UTC) #40
jfernandez
PatchSet 13 and 14 are the final agreed solution after discussing it with @timloh . ...
10 months, 1 week ago (2016-05-20 13:14:09 UTC) #41
jfernandez
10 months, 1 week ago (2016-05-20 17:03:57 UTC) #45
Timothy Loh
https://codereview.chromium.org/1709963002/diff/320001/third_party/WebKit/LayoutTests/fast/alignment/ensure-flexbox-compatibility-with-initial-values-expected.txt File third_party/WebKit/LayoutTests/fast/alignment/ensure-flexbox-compatibility-with-initial-values-expected.txt (right): https://codereview.chromium.org/1709963002/diff/320001/third_party/WebKit/LayoutTests/fast/alignment/ensure-flexbox-compatibility-with-initial-values-expected.txt#newcode6 third_party/WebKit/LayoutTests/fast/alignment/ensure-flexbox-compatibility-with-initial-values-expected.txt:6: PASS CSS.supports('align-items', 'normal') is true Is this test still ...
10 months, 1 week ago (2016-05-23 05:45:40 UTC) #46
jfernandez
Submitted a new patch with the changes suggested in the last review. https://codereview.chromium.org/1709963002/diff/320001/third_party/WebKit/LayoutTests/fast/alignment/ensure-flexbox-compatibility-with-initial-values-expected.txt File third_party/WebKit/LayoutTests/fast/alignment/ensure-flexbox-compatibility-with-initial-values-expected.txt ...
10 months, 1 week ago (2016-05-25 18:26:49 UTC) #47
jfernandez
PatchSet 17 causes 2 layout tests to fail, precisely some FullScreen cases. the FullScreen layout ...
10 months, 1 week ago (2016-05-25 23:19:16 UTC) #50
Timothy Loh
On 2016/05/25 23:19:16, jfernandez wrote: > PatchSet 17 causes 2 layout tests to fail, precisely ...
10 months, 1 week ago (2016-05-26 04:26:36 UTC) #51
jfernandez
On 2016/05/26 04:26:36, Timothy Loh wrote: > On 2016/05/25 23:19:16, jfernandez wrote: > > PatchSet ...
10 months, 1 week ago (2016-05-26 08:32:04 UTC) #52
Timothy Loh
On 2016/05/26 08:32:04, jfernandez wrote: > On 2016/05/26 04:26:36, Timothy Loh wrote: > > On ...
10 months, 1 week ago (2016-05-26 08:50:48 UTC) #53
jfernandez
On 2016/05/26 08:50:48, Timothy Loh wrote: > On 2016/05/26 08:32:04, jfernandez wrote: > > On ...
10 months, 1 week ago (2016-05-26 17:21:29 UTC) #54
jfernandez
On 2016/05/26 17:21:29, jfernandez wrote: > However, I wonder whether this approach may solve other ...
10 months, 1 week ago (2016-05-26 19:21:40 UTC) #55
Timothy Loh
foolip@: Do you know about LayoutFullScreen modifications to the layout tree or who might know ...
10 months ago (2016-05-30 06:05:02 UTC) #56
foolip_UTC9
On 2016/05/30 06:05:02, Timothy Loh wrote: > foolip@: Do you know about LayoutFullScreen modifications to ...
10 months ago (2016-05-30 06:39:17 UTC) #57
jfernandez
On 2016/05/30 06:39:17, Philip Jägenstedt wrote: > On 2016/05/30 06:05:02, Timothy Loh wrote: > > ...
10 months ago (2016-05-30 07:50:40 UTC) #58
rune
On 2016/05/30 06:05:02, Timothy Loh wrote: > foolip@: Do you know about LayoutFullScreen modifications to ...
10 months ago (2016-05-30 07:54:36 UTC) #59
rune
https://codereview.chromium.org/1709963002/diff/400001/third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp File third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp (right): https://codereview.chromium.org/1709963002/diff/400001/third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp#newcode289 third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp:289: // any legacy keywords), or 'normal' if the box ...
10 months ago (2016-05-30 07:55:10 UTC) #61
jfernandez
10 months ago (2016-05-30 10:10:43 UTC) #62
jfernandez
On 2016/05/30 07:54:36, rune wrote: > On 2016/05/30 06:05:02, Timothy Loh wrote: > > foolip@: ...
10 months ago (2016-05-30 10:12:09 UTC) #63
rune
On 2016/05/30 10:12:09, jfernandez wrote: > align-items property. Just that simple. The computed style logic ...
10 months ago (2016-05-30 12:02:10 UTC) #64
jfernandez
On 2016/05/30 12:02:10, rune wrote: > On 2016/05/30 10:12:09, jfernandez wrote: > > > align-items ...
10 months ago (2016-06-01 08:32:19 UTC) #65
Timothy Loh
On 2016/05/30 07:50:40, jfernandez wrote: > On 2016/05/30 06:39:17, Philip Jägenstedt wrote: > > On ...
10 months ago (2016-06-02 06:14:59 UTC) #66
jfernandez
On 2016/06/02 06:14:59, Timothy Loh wrote: > On 2016/05/30 07:50:40, jfernandez wrote: > > On ...
9 months, 4 weeks ago (2016-06-03 10:49:40 UTC) #67
rune
On 2016/06/03 10:49:40, jfernandez wrote: > On 2016/06/02 06:14:59, Timothy Loh wrote: > > On ...
9 months, 4 weeks ago (2016-06-03 11:28:28 UTC) #68
jfernandez
On 2016/06/03 11:28:28, rune wrote: > On 2016/06/03 10:49:40, jfernandez wrote: > > On 2016/06/02 ...
9 months, 4 weeks ago (2016-06-03 11:41:06 UTC) #69
jfernandez
PatchSet 18 contain the hack, at least from point if view it indeed is, that ...
9 months, 4 weeks ago (2016-06-04 11:40:47 UTC) #70
jfernandez
@rune @timloh any additional feedback on this ?
9 months, 2 weeks ago (2016-06-13 09:39:25 UTC) #71
rune
https://codereview.chromium.org/1709963002/diff/440001/third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp File third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp (right): https://codereview.chromium.org/1709963002/diff/440001/third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp#newcode249 third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp:249: adjustStyleForAlignment(style, parentStyle); I still have a hard time understanding ...
9 months, 2 weeks ago (2016-06-13 12:24:20 UTC) #72
jfernandez
https://codereview.chromium.org/1709963002/diff/440001/third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp File third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp (right): https://codereview.chromium.org/1709963002/diff/440001/third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp#newcode249 third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp:249: adjustStyleForAlignment(style, parentStyle); On 2016/06/13 12:24:19, rune wrote: > I ...
9 months, 2 weeks ago (2016-06-13 13:00:36 UTC) #73
jfernandez
Hi @rune and @timloh Could you please take a look at the last patch ? ...
9 months, 1 week ago (2016-06-21 13:50:33 UTC) #75
rune
On 2016/06/21 13:50:33, jfernandez wrote: > Hi @rune and @timloh Could you please take a ...
9 months, 1 week ago (2016-06-23 09:48:38 UTC) #76
rune
On 2016/06/23 09:48:38, rune wrote: > On 2016/06/21 13:50:33, jfernandez wrote: > > Hi @rune ...
9 months, 1 week ago (2016-06-23 11:54:28 UTC) #77
jfernandez
Hi @timloh, even though @rune already approved the change, could you please take a final ...
9 months ago (2016-06-28 08:40:54 UTC) #78
Timothy Loh
On 2016/06/28 08:40:54, jfernandez wrote: > Hi @timloh, even though @rune already approved the change, ...
9 months ago (2016-06-28 08:44:24 UTC) #79
Timothy Loh
lgtm (I didn't look closely at the core/layout code) https://codereview.chromium.org/1709963002/diff/540001/third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h File third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h (right): https://codereview.chromium.org/1709963002/diff/540001/third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h#newcode4341 third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h:4341: ...
9 months ago (2016-06-29 02:00:32 UTC) #80
jfernandez
https://codereview.chromium.org/1709963002/diff/540001/third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h File third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h (right): https://codereview.chromium.org/1709963002/diff/540001/third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h#newcode4341 third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h:4341: m_value.valueID = CSSValueAuto; On 2016/06/29 02:00:32, Timothy Loh wrote: ...
9 months ago (2016-06-29 19:55:47 UTC) #81
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/1709963002/560001
9 months ago (2016-06-29 21:02:37 UTC) #84
commit-bot: I haz the power
Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm64_dbg_recipe/builds/89343) android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, ...
9 months ago (2016-06-29 21:08:59 UTC) #86
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/1709963002/560001
9 months ago (2016-06-29 21:17:19 UTC) #88
commit-bot: I haz the power
Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm64_dbg_recipe/builds/89358) android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, ...
9 months ago (2016-06-29 21:24:30 UTC) #90
Timothy Loh
https://codereview.chromium.org/1709963002/diff/540001/third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h File third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h (right): https://codereview.chromium.org/1709963002/diff/540001/third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h#newcode4341 third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h:4341: m_value.valueID = CSSValueAuto; On 2016/06/29 19:55:47, jfernandez wrote: > ...
9 months ago (2016-06-30 01:50:05 UTC) #91
jfernandez
https://codereview.chromium.org/1709963002/diff/540001/third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h File third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h (right): https://codereview.chromium.org/1709963002/diff/540001/third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h#newcode4341 third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h:4341: m_value.valueID = CSSValueAuto; On 2016/06/30 01:50:05, Timothy Loh wrote: ...
9 months ago (2016-06-30 10:17:53 UTC) #92
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/1709963002/580001
9 months ago (2016-06-30 10:20:05 UTC) #95
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_ozone_rel_ng/builds/194697)
9 months ago (2016-06-30 11:13:07 UTC) #97
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/1709963002/600001
9 months ago (2016-06-30 12:51:43 UTC) #100
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/253150)
9 months ago (2016-06-30 13:39:01 UTC) #102
jfernandez
Hi @cbiesinger , could you help me to understand why the StyleAdjuster approach we finally ...
8 months, 2 weeks ago (2016-07-12 09:15:05 UTC) #107
cbiesinger
On 2016/07/12 09:15:05, jfernandez wrote: > Hi @cbiesinger , could you help me to understand ...
8 months, 2 weeks ago (2016-07-13 21:13:23 UTC) #108
jfernandez
On 2016/07/13 21:13:23, cbiesinger wrote: > On 2016/07/12 09:15:05, jfernandez wrote: > > Hi @cbiesinger ...
8 months, 2 weeks ago (2016-07-13 21:27:13 UTC) #109
cbiesinger
On 2016/07/13 21:27:13, jfernandez wrote: > On 2016/07/13 21:13:23, cbiesinger wrote: > > On 2016/07/12 ...
8 months, 2 weeks ago (2016-07-13 21:33:49 UTC) #110
jfernandez
On 2016/07/13 21:33:49, cbiesinger wrote: > On 2016/07/13 21:27:13, jfernandez wrote: > > On 2016/07/13 ...
8 months, 2 weeks ago (2016-07-13 21:40:58 UTC) #111
jfernandez
I think I finally managed to solve the issues with the InputFieldType elements and shadowDOM; ...
8 months, 1 week ago (2016-07-20 13:37:45 UTC) #112
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/1709963002/820001
7 months, 2 weeks ago (2016-08-17 23:53:03 UTC) #116
commit-bot: I haz the power
Committed patchset #33 (id:820001)
7 months, 2 weeks ago (2016-08-18 03:55:33 UTC) #118
commit-bot: I haz the power
7 months, 2 weeks ago (2016-08-18 03:58:25 UTC) #120
Message was sent while issue was closed.
Patchset 33 (id:??) landed as
https://crrev.com/fe59cb90e7b08f1faa146088ddbd7589d87eebb5
Cr-Commit-Position: refs/heads/master@{#412728}
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cc6ac46