|
Description
Sergio Villar Senin
2015-06-16 08:40:37 PDT
We need to merge these 3: - https://crrev.com/193665 - https://crrev.com/194062 - https://crrev.com/195930 and part of this one: https://crrev.com/194887 Created attachment 255497 [details]
Patch
Comment on attachment 255497 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=255497&action=review I’m not the right reviewer for this. Might need to be Hyatt. > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1857 > +inline static bool isFlexOrGrid(Node* element) Type could be ContainerNode*. Comment on attachment 255497 [details] Patch Attachment 255497 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6454581009580032 New failing tests: mathml/presentation/menclose-notation-default-longdiv.html Created attachment 255499 [details]
Archive of layout-test-results from ews102 for mac-mavericks
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102 Port: mac-mavericks Platform: Mac OS X 10.9.5
Comment on attachment 255497 [details] Patch Attachment 255497 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6120144791142400 New failing tests: mathml/presentation/menclose-notation-default-longdiv.html Created attachment 255500 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Created attachment 255639 [details]
Patch
This new version ensures that the new policy is not applied to RenderFlexibleBox derived classes. Also includes the removal of some hacks added to properly shrink some elements
Comment on attachment 255639 [details] Patch Attachment 255639 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5371899748024320 New failing tests: fast/css/auto-min-size.html Created attachment 255642 [details]
Archive of layout-test-results from ews100 for mac-mavericks
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100 Port: mac-mavericks Platform: Mac OS X 10.9.5
Comment on attachment 255639 [details] Patch Attachment 255639 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5107354190217216 New failing tests: fast/css/auto-min-size.html Created attachment 255644 [details]
Archive of layout-test-results from ews106 for mac-mavericks-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Created attachment 255918 [details]
Patch
Comment on attachment 255918 [details] Patch Attachment 255918 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5074698748559360 New failing tests: fast/css/auto-min-size.html Created attachment 255921 [details]
Archive of layout-test-results from ews102 for mac-mavericks
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102 Port: mac-mavericks Platform: Mac OS X 10.9.5
Comment on attachment 255918 [details] Patch Attachment 255918 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5477820083994624 New failing tests: fast/css/auto-min-size.html Created attachment 255922 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Created attachment 255998 [details]
Patch
Hyatt maybe? Comment on attachment 255998 [details]
Patch
r=me
Committed r189536: <http://trac.webkit.org/changeset/189536> This caused 4 tests to fail everywhere on mac. Please check before relanding. Before: https://build.webkit.org/results/Apple%20Yosemite%20Release%20WK2%20(Tests)/r189535%20(8182)/results.html After: https://build.webkit.org/results/Apple%20Yosemite%20Release%20WK2%20(Tests)/r189536%20(8183)/results.html Newly failing tests: fast/css/auto-min-size.html +fast/forms/menulist-narrow-width.html +fast/replaced/width100percent-menulist.html +fast/replaced/width100percent-searchfield.html Re-opened since this is blocked by bug 149002 Created attachment 260916 [details]
Patch for landing
Removed some code specific for the gtk port that somehow got into the final patch for review
Created attachment 260917 [details]
Patch for landing v2
Removed some code specific for the gtk port that somehow got into the final patch for review
Comment on attachment 260917 [details]
Patch for landing v2
Do not set r? as it was reviewed previously
Committed r189567: <http://trac.webkit.org/changeset/189567> This broke various internal tools at Apple, and other companies, which now expand a flex box vertically instead of showing overflow-scroll. Were there any follow-up fixes in Blink? (In reply to comment #28) > This broke various internal tools at Apple, and other companies, which now > expand a flex box vertically instead of showing overflow-scroll. Were there > any follow-up fixes in Blink? Flexbox code in Blink has been subject to tons of changes lately. I can try to bisect the fix for that issue but I'd need a reduced test case. Could you try to cook one? (In reply to comment #28) > This broke various internal tools at Apple, and other companies, which now > expand a flex box vertically instead of showing overflow-scroll. Were there > any follow-up fixes in Blink? BTW just in case, as the ChangeLog says, many use cases behave the same just by adding min-width|height: 0px which was the previous default value. Examples appear to include: 156786, 157698 Of the other bugs I attached, it looks like we definitely behave differently from Firefox and Chrome with 156786 and 157678. The others might be evangelism or behaves correctly. It seems that under the following conditions: - the flex container has flex-direction:column and overflow:auto/scroll; - the flex item is a flex container itself with flex-direction:row the default min-height:auto still doesn't work correctly (works as 0 instead of the content height). Here is a small self-explaining testcase demonstrating the issue: https://jsfiddle.net/9xzhb6m4/2/ (In reply to Ilya Streltsyn from comment #34) > It seems that under the following conditions: > > - the flex container has flex-direction:column and overflow:auto/scroll; > - the flex item is a flex container itself with flex-direction:row > > the default min-height:auto still doesn't work correctly (works as 0 instead > of the content height). Here is a small self-explaining testcase > demonstrating the issue: https://jsfiddle.net/9xzhb6m4/2/ Would you be willing to file a new bug report? Or is there some reason that a comment on this existing report is a better way to handle this? (In reply to Ilya Streltsyn from comment #34) > It seems that under the following conditions: > > - the flex container has flex-direction:column and overflow:auto/scroll; > - the flex item is a flex container itself with flex-direction:row > > the default min-height:auto still doesn't work correctly (works as 0 instead > of the content height). Here is a small self-explaining testcase > demonstrating the issue: https://jsfiddle.net/9xzhb6m4/2/ Based on the jsfiddle, it looks like we currently agree with Chrome but disagree with Firefox. Ilya, please file a new bug citing this example. |