Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1421423005:
[css-flexbox] Fix min-size: auto for a non-auto flex basis (Closed)
|
Created:
5 years, 1 month ago by cbiesinger Modified:
5 years, 1 month ago CC:
blink-reviews, blink-reviews-layout_chromium.org, chromium-reviews, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, pdr+renderingwatchlist_chromium.org, szager+layoutwatch_chromium.org, zoltan1 Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[css-flexbox] Fix min-size: auto for a non-auto flex basis
[IF THIS BREAKS ANY CONTENT/CHROME UI: Please do not revert this change.
Instead, add "min-width: 0; min-height: 0; to your flex items, which
should fix any issues that may occur]
In https://codereview.chromium.org/1098593002 I fixed a perf regression
in an incorrect way, leading to a correctness in certain circumstances.
This patch fixes the correctness issue while still fixing the performance
issue, by re-using the cached intrinsic width in most cases.
Specifically, the optimization was assuming that we only
need to apply min-width: auto if we applied flex-shrink.
However, that is not correct. We also need to apply it if we
have a flex-basis that's less than the min-content width, for
example (most common case) flex-basis: 0. There might be further
cases that could lead us to calculate a width less than min-width
that I can't think of right now, so this is the safest way
to fix this.
Local performance data:
WITH THIS PATCH:
RESULT flexbox-with-stretch-layout: flexbox-with-stretch-layout.html= [15.430194,14.878406,15.044268,15.965514,14.611712] runs/s
Avg flexbox-with-stretch-layout: 15.186019runs/s
Sd flexbox-with-stretch-layout: 0.527205runs/s
BEFORE THIS PATCH:
RESULT flexbox-with-stretch-layout: flexbox-with-stretch-layout.html= [12.513922,12.225014,9.769322,10.982909,10.422027] runs/s
Avg flexbox-with-stretch-layout: 11.182639runs/s
Sd flexbox-with-stretch-layout: 1.169909runs/s
BEFORE THE ORIGINAL PERF FIX:
RESULT flexbox-with-stretch-layout: flexbox-with-stretch-layout.html= [9.750093,9.870511,9.825113,10.636098,9.464543] runs/s
Avg flexbox-with-stretch-layout: 9.909272runs/s
Sd flexbox-with-stretch-layout: 0.435885runs/s
Adding all potentially related bugs to the BUG list here. I have not verified
that all of them are indeed this specific issue.
R=leviw@chromium.org
BUG=546034, 477183, 504931, 504391, 491076
Committed: https://crrev.com/8b2c57bf0ac6c22f5a4d593d326f5f239b7d5808
Cr-Commit-Position: refs/heads/master@{#356667}
Patch Set 1 #
Total comments: 4
Patch Set 2 : review comments #Patch Set 3 : fix tests #
Total comments: 1
Patch Set 4 : had to update some plugin placeholder files as well #
Total comments: 6
Patch Set 5 : review comments #
Messages
Total messages: 40 (16 generated)
The CQ bit was checked by cbiesinger@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1421423005/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1421423005/1
https://codereview.chromium.org/1421423005/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp (right): https://codereview.chromium.org/1421423005/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp:452: if (size.type() == MinContent && styleRef().logicalWidth().isAuto()) if (styleRef().logicalWidth().isAuto) { if (size.type() == MinContent) return ... else if (... } https://codereview.chromium.org/1421423005/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/layout/LayoutFlexibleBox.h (left): https://codereview.chromium.org/1421423005/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/LayoutFlexibleBox.h:153: LayoutUnit adjustChildSizeForMinAndMax(const LayoutBox& child, LayoutUnit childSize, bool childShrunk = false); Can you explain what exactly was going wrong here?
Description was changed from ========== [css-flexbox] Fix min-size: auto for a non-auto flex basis In https://codereview.chromium.org/1098593002 I fixed a perf regression in an incorrect way, leading to a correctness in certain circumstances. This patch fixes the correctness issue while still fixing the performance issue, by re-using the cached intrinsic width in most cases. Local performance data: WITH THIS PATCH: RESULT flexbox-with-stretch-layout: flexbox-with-stretch-layout.html= [15.430194,14.878406,15.044268,15.965514,14.611712] runs/s Avg flexbox-with-stretch-layout: 15.186019runs/s Sd flexbox-with-stretch-layout: 0.527205runs/s BEFORE THIS PATCH: RESULT flexbox-with-stretch-layout: flexbox-with-stretch-layout.html= [12.513922,12.225014,9.769322,10.982909,10.422027] runs/s Avg flexbox-with-stretch-layout: 11.182639runs/s Sd flexbox-with-stretch-layout: 1.169909runs/s BEFORE THE ORIGINAL PERF FIX: RESULT flexbox-with-stretch-layout: flexbox-with-stretch-layout.html= [9.750093,9.870511,9.825113,10.636098,9.464543] runs/s Avg flexbox-with-stretch-layout: 9.909272runs/s Sd flexbox-with-stretch-layout: 0.435885runs/s Adding all potentially related bugs to the BUG list here. I have not verified that all of them are indeed this specific issue. R=leviw@chromium.org BUG=546034,477183,504931,504391 ========== to ========== [css-flexbox] Fix min-size: auto for a non-auto flex basis In https://codereview.chromium.org/1098593002 I fixed a perf regression in an incorrect way, leading to a correctness in certain circumstances. This patch fixes the correctness issue while still fixing the performance issue, by re-using the cached intrinsic width in most cases. Specifically, the optimization was assuming that we only need to apply min-width: auto if we applied flex-shrink. However, that is not correct. We also need to apply it if we have a flex-basis that's less than the min-content width, for example (most common case) flex-basis: 0. There might be further cases that could lead us to calculate a width less than min-width that I can't think of right now, so this is the safest way to fix this. Local performance data: WITH THIS PATCH: RESULT flexbox-with-stretch-layout: flexbox-with-stretch-layout.html= [15.430194,14.878406,15.044268,15.965514,14.611712] runs/s Avg flexbox-with-stretch-layout: 15.186019runs/s Sd flexbox-with-stretch-layout: 0.527205runs/s BEFORE THIS PATCH: RESULT flexbox-with-stretch-layout: flexbox-with-stretch-layout.html= [12.513922,12.225014,9.769322,10.982909,10.422027] runs/s Avg flexbox-with-stretch-layout: 11.182639runs/s Sd flexbox-with-stretch-layout: 1.169909runs/s BEFORE THE ORIGINAL PERF FIX: RESULT flexbox-with-stretch-layout: flexbox-with-stretch-layout.html= [9.750093,9.870511,9.825113,10.636098,9.464543] runs/s Avg flexbox-with-stretch-layout: 9.909272runs/s Sd flexbox-with-stretch-layout: 0.435885runs/s Adding all potentially related bugs to the BUG list here. I have not verified that all of them are indeed this specific issue. R=leviw@chromium.org BUG=546034,477183,504931,504391 ==========
The CQ bit was checked by cbiesinger@chromium.org to run a CQ dry run
https://codereview.chromium.org/1421423005/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp (right): https://codereview.chromium.org/1421423005/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp:452: if (size.type() == MinContent && styleRef().logicalWidth().isAuto()) On 2015/10/28 01:24:56, leviw wrote: > if (styleRef().logicalWidth().isAuto) { > if (size.type() == MinContent) > return ... > else if (... > } Done, except for the "else if" part -- the style guide says to just use if after a return (linebreaking-else-if in https://www.chromium.org/blink/coding-style) https://codereview.chromium.org/1421423005/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/layout/LayoutFlexibleBox.h (left): https://codereview.chromium.org/1421423005/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/LayoutFlexibleBox.h:153: LayoutUnit adjustChildSizeForMinAndMax(const LayoutBox& child, LayoutUnit childSize, bool childShrunk = false); On 2015/10/28 01:24:57, leviw wrote: > Can you explain what exactly was going wrong here? I changed the description of the patch to explain it, see there.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1421423005/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1421423005/20001
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
On 2015/10/28 02:39:39, commit-bot: I haz the power wrote: > Dry run: Try jobs failed on following builders: > linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED, > http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) ASSERTION FAILED: contentSize >= 0 in browser tests one day, I want to make a nontrivial flexbox change that does not cause random tests to fail :(
The CQ bit was checked by cbiesinger@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1421423005/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1421423005/40001
(also min-size-auto.html needed new expectations because there are more PASS lines now) https://codereview.chromium.org/1421423005/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp (left): https://codereview.chromium.org/1421423005/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp:1150: if (childFlexBasis.isIntrinsic() || childMinSize.isIntrinsic() || childMaxSize.isIntrinsic()) I had to make this change here. A min-size of auto in this condition does require us to lay out the child just like a min-size of min-content would.
The CQ bit was unchecked by cbiesinger@chromium.org
The CQ bit was checked by cbiesinger@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from leviw@chromium.org Link to the patchset: https://codereview.chromium.org/1421423005/#ps40001 (title: "fix tests")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1421423005/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1421423005/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by cbiesinger@chromium.org to run a CQ dry run
cbiesinger@chromium.org changed reviewers: + bauerb@chromium.org, tommycli@chromium.org
Bernhard, Tommy - could one of you give me an lgtm for the chrome/renderer/resources/plugins files? Thanks!
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1421423005/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1421423005/60001
Description was changed from ========== [css-flexbox] Fix min-size: auto for a non-auto flex basis In https://codereview.chromium.org/1098593002 I fixed a perf regression in an incorrect way, leading to a correctness in certain circumstances. This patch fixes the correctness issue while still fixing the performance issue, by re-using the cached intrinsic width in most cases. Specifically, the optimization was assuming that we only need to apply min-width: auto if we applied flex-shrink. However, that is not correct. We also need to apply it if we have a flex-basis that's less than the min-content width, for example (most common case) flex-basis: 0. There might be further cases that could lead us to calculate a width less than min-width that I can't think of right now, so this is the safest way to fix this. Local performance data: WITH THIS PATCH: RESULT flexbox-with-stretch-layout: flexbox-with-stretch-layout.html= [15.430194,14.878406,15.044268,15.965514,14.611712] runs/s Avg flexbox-with-stretch-layout: 15.186019runs/s Sd flexbox-with-stretch-layout: 0.527205runs/s BEFORE THIS PATCH: RESULT flexbox-with-stretch-layout: flexbox-with-stretch-layout.html= [12.513922,12.225014,9.769322,10.982909,10.422027] runs/s Avg flexbox-with-stretch-layout: 11.182639runs/s Sd flexbox-with-stretch-layout: 1.169909runs/s BEFORE THE ORIGINAL PERF FIX: RESULT flexbox-with-stretch-layout: flexbox-with-stretch-layout.html= [9.750093,9.870511,9.825113,10.636098,9.464543] runs/s Avg flexbox-with-stretch-layout: 9.909272runs/s Sd flexbox-with-stretch-layout: 0.435885runs/s Adding all potentially related bugs to the BUG list here. I have not verified that all of them are indeed this specific issue. R=leviw@chromium.org BUG=546034,477183,504931,504391 ========== to ========== [css-flexbox] Fix min-size: auto for a non-auto flex basis In https://codereview.chromium.org/1098593002 I fixed a perf regression in an incorrect way, leading to a correctness in certain circumstances. This patch fixes the correctness issue while still fixing the performance issue, by re-using the cached intrinsic width in most cases. Specifically, the optimization was assuming that we only need to apply min-width: auto if we applied flex-shrink. However, that is not correct. We also need to apply it if we have a flex-basis that's less than the min-content width, for example (most common case) flex-basis: 0. There might be further cases that could lead us to calculate a width less than min-width that I can't think of right now, so this is the safest way to fix this. Local performance data: WITH THIS PATCH: RESULT flexbox-with-stretch-layout: flexbox-with-stretch-layout.html= [15.430194,14.878406,15.044268,15.965514,14.611712] runs/s Avg flexbox-with-stretch-layout: 15.186019runs/s Sd flexbox-with-stretch-layout: 0.527205runs/s BEFORE THIS PATCH: RESULT flexbox-with-stretch-layout: flexbox-with-stretch-layout.html= [12.513922,12.225014,9.769322,10.982909,10.422027] runs/s Avg flexbox-with-stretch-layout: 11.182639runs/s Sd flexbox-with-stretch-layout: 1.169909runs/s BEFORE THE ORIGINAL PERF FIX: RESULT flexbox-with-stretch-layout: flexbox-with-stretch-layout.html= [9.750093,9.870511,9.825113,10.636098,9.464543] runs/s Avg flexbox-with-stretch-layout: 9.909272runs/s Sd flexbox-with-stretch-layout: 0.435885runs/s Adding all potentially related bugs to the BUG list here. I have not verified that all of them are indeed this specific issue. R=leviw@chromium.org BUG=546034,477183,504931,504391,491076 ==========
Description was changed from ========== [css-flexbox] Fix min-size: auto for a non-auto flex basis In https://codereview.chromium.org/1098593002 I fixed a perf regression in an incorrect way, leading to a correctness in certain circumstances. This patch fixes the correctness issue while still fixing the performance issue, by re-using the cached intrinsic width in most cases. Specifically, the optimization was assuming that we only need to apply min-width: auto if we applied flex-shrink. However, that is not correct. We also need to apply it if we have a flex-basis that's less than the min-content width, for example (most common case) flex-basis: 0. There might be further cases that could lead us to calculate a width less than min-width that I can't think of right now, so this is the safest way to fix this. Local performance data: WITH THIS PATCH: RESULT flexbox-with-stretch-layout: flexbox-with-stretch-layout.html= [15.430194,14.878406,15.044268,15.965514,14.611712] runs/s Avg flexbox-with-stretch-layout: 15.186019runs/s Sd flexbox-with-stretch-layout: 0.527205runs/s BEFORE THIS PATCH: RESULT flexbox-with-stretch-layout: flexbox-with-stretch-layout.html= [12.513922,12.225014,9.769322,10.982909,10.422027] runs/s Avg flexbox-with-stretch-layout: 11.182639runs/s Sd flexbox-with-stretch-layout: 1.169909runs/s BEFORE THE ORIGINAL PERF FIX: RESULT flexbox-with-stretch-layout: flexbox-with-stretch-layout.html= [9.750093,9.870511,9.825113,10.636098,9.464543] runs/s Avg flexbox-with-stretch-layout: 9.909272runs/s Sd flexbox-with-stretch-layout: 0.435885runs/s Adding all potentially related bugs to the BUG list here. I have not verified that all of them are indeed this specific issue. R=leviw@chromium.org BUG=546034,477183,504931,504391,491076 ========== to ========== [css-flexbox] Fix min-size: auto for a non-auto flex basis [IF THIS BREAKS ANY CONTENT/CHROME UI: Please do not revert this change. Instead, add "min-width: 0; min-height: 0; to your flex items, which should fix any issues that may occur] In https://codereview.chromium.org/1098593002 I fixed a perf regression in an incorrect way, leading to a correctness in certain circumstances. This patch fixes the correctness issue while still fixing the performance issue, by re-using the cached intrinsic width in most cases. Specifically, the optimization was assuming that we only need to apply min-width: auto if we applied flex-shrink. However, that is not correct. We also need to apply it if we have a flex-basis that's less than the min-content width, for example (most common case) flex-basis: 0. There might be further cases that could lead us to calculate a width less than min-width that I can't think of right now, so this is the safest way to fix this. Local performance data: WITH THIS PATCH: RESULT flexbox-with-stretch-layout: flexbox-with-stretch-layout.html= [15.430194,14.878406,15.044268,15.965514,14.611712] runs/s Avg flexbox-with-stretch-layout: 15.186019runs/s Sd flexbox-with-stretch-layout: 0.527205runs/s BEFORE THIS PATCH: RESULT flexbox-with-stretch-layout: flexbox-with-stretch-layout.html= [12.513922,12.225014,9.769322,10.982909,10.422027] runs/s Avg flexbox-with-stretch-layout: 11.182639runs/s Sd flexbox-with-stretch-layout: 1.169909runs/s BEFORE THE ORIGINAL PERF FIX: RESULT flexbox-with-stretch-layout: flexbox-with-stretch-layout.html= [9.750093,9.870511,9.825113,10.636098,9.464543] runs/s Avg flexbox-with-stretch-layout: 9.909272runs/s Sd flexbox-with-stretch-layout: 0.435885runs/s Adding all potentially related bugs to the BUG list here. I have not verified that all of them are indeed this specific issue. R=leviw@chromium.org BUG=546034,477183,504931,504391,491076 ==========
lgtm sans below: I think most of the min-height/width declarations aren't needed to pass the browsertests (other than the plugin-icon) one. https://codereview.chromium.org/1421423005/diff/60001/chrome/renderer/resourc... File chrome/renderer/resources/plugins/plugin_placeholders.css (right): https://codereview.chromium.org/1421423005/diff/60001/chrome/renderer/resourc... chrome/renderer/resources/plugins/plugin_placeholders.css:44: #inner { probably not needed https://codereview.chromium.org/1421423005/diff/60001/chrome/renderer/resourc... File chrome/renderer/resources/plugins/plugin_poster.html (right): https://codereview.chromium.org/1421423005/diff/60001/chrome/renderer/resourc... chrome/renderer/resources/plugins/plugin_poster.html:49: min-width: 0; Probably not needed. https://codereview.chromium.org/1421423005/diff/60001/chrome/renderer/resourc... chrome/renderer/resources/plugins/plugin_poster.html:65: min-height: 0; Probably not needed also if plugin-icon itself is squashed small right?
https://codereview.chromium.org/1421423005/diff/60001/chrome/renderer/resourc... File chrome/renderer/resources/plugins/plugin_placeholders.css (right): https://codereview.chromium.org/1421423005/diff/60001/chrome/renderer/resourc... chrome/renderer/resources/plugins/plugin_placeholders.css:44: #inner { On 2015/10/28 21:02:01, tommycli wrote: > probably not needed ok, removed -- added this one more just to be safe. https://codereview.chromium.org/1421423005/diff/60001/chrome/renderer/resourc... File chrome/renderer/resources/plugins/plugin_poster.html (right): https://codereview.chromium.org/1421423005/diff/60001/chrome/renderer/resourc... chrome/renderer/resources/plugins/plugin_poster.html:49: min-width: 0; On 2015/10/28 21:02:01, tommycli wrote: > Probably not needed. Yeah, not needed for the tests. Removed. https://codereview.chromium.org/1421423005/diff/60001/chrome/renderer/resourc... chrome/renderer/resources/plugins/plugin_poster.html:65: min-height: 0; On 2015/10/28 21:02:01, tommycli wrote: > Probably not needed also if plugin-icon itself is squashed small right? Removed.
The CQ bit was checked by cbiesinger@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from leviw@chromium.org, tommycli@chromium.org Link to the patchset: https://codereview.chromium.org/1421423005/#ps80001 (title: "review comments")
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1421423005/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1421423005/80001
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/8b2c57bf0ac6c22f5a4d593d326f5f239b7d5808 Cr-Commit-Position: refs/heads/master@{#356667}
Message was sent while issue was closed.
dbeam@chromium.org changed reviewers: + dbeam@chromium.org
Message was sent while issue was closed.
> [IF THIS BREAKS ANY CONTENT/CHROME UI: Please do not revert this change. I don't really see how Chrome's UI is any different from the open web's in this regard (other than we get to play with the new toys sooner and have revert access). This will probably break lots of sites. Hopefully they'll have the ability to update their code in a timely manner (i.e. it's not stuck in some library or cached forever). I'll keep updating parts of Chrome that this broke (mainly the new downloads UI).
Message was sent while issue was closed.
On 2015/11/03 at 05:18:58, dbeam wrote: > > [IF THIS BREAKS ANY CONTENT/CHROME UI: Please do not revert this change. > > I don't really see how Chrome's UI is any different from the open web's in this regard (other than we get to play with the new toys sooner and have revert access). > > This will probably break lots of sites. Hopefully they'll have the ability to update their code in a timely manner (i.e. it's not stuck in some library or cached forever). > > I'll keep updating parts of Chrome that this broke (mainly the new downloads UI). This broke the new codereview site as well, but it was already broken in Firefox. So this is forcing authors to make their code work in Firefox which is nice.
Message was sent while issue was closed.
On 2015/11/03 05:31:16, esprehn wrote: > On 2015/11/03 at 05:18:58, dbeam wrote: > > > [IF THIS BREAKS ANY CONTENT/CHROME UI: Please do not revert this change. > > > > I don't really see how Chrome's UI is any different from the open web's in > this regard (other than we get to play with the new toys sooner and have revert > access). > > > > This will probably break lots of sites. Hopefully they'll have the ability to > update their code in a timely manner (i.e. it's not stuck in some library or > cached forever). > > > > I'll keep updating parts of Chrome that this broke (mainly the new downloads > UI). > > This broke the new codereview site as well, but it was already broken in > Firefox. So this is forcing authors to make their code work in Firefox which is > nice. if firefox already had this behavior I suppose it's good to be consistent.
Message was sent while issue was closed.
On 2015/11/03 05:43:18, Dan Beam wrote: > On 2015/11/03 05:31:16, esprehn wrote: > > On 2015/11/03 at 05:18:58, dbeam wrote: > > > > [IF THIS BREAKS ANY CONTENT/CHROME UI: Please do not revert this change. > > > > > > I don't really see how Chrome's UI is any different from the open web's in > > this regard (other than we get to play with the new toys sooner and have > revert > > access). > > > > > > This will probably break lots of sites. Hopefully they'll have the ability > to > > update their code in a timely manner (i.e. it's not stuck in some library or > > cached forever). > > > > > > I'll keep updating parts of Chrome that this broke (mainly the new downloads > > UI). > > > > This broke the new codereview site as well, but it was already broken in > > Firefox. So this is forcing authors to make their code work in Firefox which > is > > nice. > > if firefox already had this behavior I suppose it's good to be consistent. Yes, both Edge and Firefox have this behavior. I should probably have included that in the commit message. The other thing is that before *this* checkin, we had this weird half-implemented min-width: auto behavior which just doesn't make sense. So, I think it's best for the web to see this through and work through the breakage. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
