|
|
Created:
3 years, 9 months ago by cathiechentx Modified:
3 years, 9 months ago CC:
chromium-reviews, pdr+renderingwatchlist_chromium.org, zoltan1, blink-reviews-layout_chromium.org, szager+layoutwatch_chromium.org, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering, blink-reviews Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionGet markers back while list items have overflow:hidden children
Add marker into list item not overflow:hidden child while construct
layout tree.
BUG=626293
Review-Url: https://codereview.chromium.org/2759883002
Cr-Commit-Position: refs/heads/master@{#459372}
Committed: https://chromium.googlesource.com/chromium/src/+/fd1e60f0bee4b6b25d898f4f4f859aa38d1a9643
Patch Set 1 #Patch Set 2 : add layout test case #
Total comments: 12
Patch Set 3 : use reftest instead #Patch Set 4 : omit addLogicalHeightFromChild() #
Messages
Total messages: 41 (17 generated)
Description was changed from ========== Get markers back, while listitems have overflow:hidden children BUG=626293 ========== to ========== Get markers back while listitems have overflow:hidden children BUG=626293 ==========
Description was changed from ========== Get markers back while listitems have overflow:hidden children BUG=626293 ========== to ========== Get markers back while list items have overflow:hidden children 1. Add marker into list item not overflow:hidden child while construct layout tree. 2. In order to keep pace with common list item which position its marker on the left, do not let marker effect the height of list item while it's layouting block children. BUG=626293 ==========
Description was changed from ========== Get markers back while list items have overflow:hidden children 1. Add marker into list item not overflow:hidden child while construct layout tree. 2. In order to keep pace with common list item which position its marker on the left, do not let marker effect the height of list item while it's layouting block children. BUG=626293 ========== to ========== Get markers back while list items have overflow:hidden children 1. Add marker into list item not overflow:hidden child while construct layout tree. 2. In order to stay consistent with common list item which position its marker on the left, do not let marker effect the height of list item while it's layouting block children. BUG=626293 ==========
charlieiflo@gmail.com changed reviewers: + charlieiflo@gmail.com
cathiechen@tencent.com changed reviewers: + eae@chromium.org
Hi, Do you guys have time to look at this. Thanks:)
mstensho@opera.com changed reviewers: + mstensho@opera.com
You should wrap the text in the commit comment at 80 chars or so. I'm looking at the line that starts with "2. In order to stay [...]". I'm suggesting that you revert that part of the patch anyway, though, so if you agree, you can just delete that line. https://codereview.chromium.org/2759883002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/TestExpectations (right): https://codereview.chromium.org/2759883002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/TestExpectations:774: crbug.com/626293 fast/lists/list-marker-before-overflow-hidden.htm [ NeedsRebaseline ] There are ways (webkit-patch rebaseline-cl) to rebaseline on your own as part of this CL. But: I suggest that you write a reftest instead. That should be trivial in this case. https://codereview.chromium.org/2759883002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/lists/list-marker-before-overflow-hidden.htm (right): https://codereview.chromium.org/2759883002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/lists/list-marker-before-overflow-hidden.htm:1: <!DOCTYPE html> File should be named .html, not .htm https://codereview.chromium.org/2759883002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/lists/list-marker-before-overflow-hidden.htm:2: <p>crbug.com/626293: List marks shoudn't be missed while li have a overflow:hidden child.</p> *markers *shouldn't Or, how about just saying: "There should be a list item marker below."? https://codereview.chromium.org/2759883002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp (right): https://codereview.chromium.org/2759883002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp:890: toLayoutListItem(this)->addLogicalHeightFromChild(child); Without this special-code, the list item marker would occupy a line on its own, and here's how you attempt to avoid it. But I'm not sure if you really have to do that. Edge (but not Firefox) renders your test just like that, i.e. with a marker on a separate line, followed by the overflow:hidden block. So I suggest you do that too (i.e just back out this change), unless there's a spec that requires us to behave differently. If we can keep it simple, we should do that. :)
Description was changed from ========== Get markers back while list items have overflow:hidden children 1. Add marker into list item not overflow:hidden child while construct layout tree. 2. In order to stay consistent with common list item which position its marker on the left, do not let marker effect the height of list item while it's layouting block children. BUG=626293 ========== to ========== Get markers back while list items have overflow:hidden children 1. Add marker into list item not overflow:hidden child while construct layout tree. 2. In order to stay consistent with common list item which position its marker on the left, do not let marker effect the height of list item while it's layouting block children. BUG=626293 ==========
Thanks for reviewing:) And sorry about the long description... https://codereview.chromium.org/2759883002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/TestExpectations (right): https://codereview.chromium.org/2759883002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/TestExpectations:774: crbug.com/626293 fast/lists/list-marker-before-overflow-hidden.htm [ NeedsRebaseline ] On 2017/03/20 10:19:00, mstensho wrote: > There are ways (webkit-patch rebaseline-cl) to rebaseline on your own as part of > this CL. > > But: I suggest that you write a reftest instead. That should be trivial in this > case. Done. https://codereview.chromium.org/2759883002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/lists/list-marker-before-overflow-hidden.htm (right): https://codereview.chromium.org/2759883002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/lists/list-marker-before-overflow-hidden.htm:1: <!DOCTYPE html> On 2017/03/20 10:19:01, mstensho wrote: > File should be named .html, not .htm Done. https://codereview.chromium.org/2759883002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/lists/list-marker-before-overflow-hidden.htm:2: <p>crbug.com/626293: List marks shoudn't be missed while li have a overflow:hidden child.</p> On 2017/03/20 10:19:01, mstensho wrote: > *markers > *shouldn't > > Or, how about just saying: "There should be a list item marker below."? Done. https://codereview.chromium.org/2759883002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp (right): https://codereview.chromium.org/2759883002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp:890: toLayoutListItem(this)->addLogicalHeightFromChild(child); On 2017/03/20 10:19:01, mstensho wrote: > Without this special-code, the list item marker would occupy a line on its own, > and here's how you attempt to avoid it. But I'm not sure if you really have to > do that. Edge (but not Firefox) renders your test just like that, i.e. with a > marker on a separate line, followed by the overflow:hidden block. > > So I suggest you do that too (i.e just back out this change), unless there's a > spec that requires us to behave differently. If we can keep it simple, we should > do that. :) Yeah, that make sense. But I'm concerned about the difference between overflow:hidden and overflow:visible might confuse people. With overflow:hidden child we change it from no list marker to occupied one-line list marker. It doesn't seem to have improved.
https://codereview.chromium.org/2759883002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp (right): https://codereview.chromium.org/2759883002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp:890: toLayoutListItem(this)->addLogicalHeightFromChild(child); On 2017/03/20 12:56:44, cathiechentx wrote: > On 2017/03/20 10:19:01, mstensho wrote: > > Without this special-code, the list item marker would occupy a line on its > own, > > and here's how you attempt to avoid it. But I'm not sure if you really have to > > do that. Edge (but not Firefox) renders your test just like that, i.e. with a > > marker on a separate line, followed by the overflow:hidden block. > > > > So I suggest you do that too (i.e just back out this change), unless there's a > > spec that requires us to behave differently. If we can keep it simple, we > should > > do that. :) > > Yeah, that make sense. But I'm concerned about the difference between > overflow:hidden and overflow:visible might confuse people. With overflow:hidden > child we change it from no list marker to occupied one-line list marker. It > doesn't seem to have improved. Looks like Firefox doesn't really put the marker next to the line box - it just fakes it somehow: <!DOCTYPE html> <ul> <li> <div style="position:relative; top:100px;"> ←Marker to the left here? </div> </li> </ul> In Firefox, the list item marker stays put, while the DIV and the line of text are moved down by 100px. Edge and Blink render it identically, with the marker next to the DIV. The new code inside LayoutListItem (invoked from here) tries to make it appear as if the marker is part of the line inside the DIV by kind of taking the marker out of flow. It falls apart as soon as one does something like this, though: <!DOCTYPE html> <ul> <li> <div style="overflow:hidden; margin-top:100px;"> ←Is the marker still there? </div> </li> </ul> Or this: <!DOCTYPE html> <ul> <li> <div style="overflow:hidden;"> ←Marker baseline alignment <span style="font-size:5em;">FAIL</span> </div> </li> </ul> Here the marker aligns itself with an imaginary line inside the hotpink block: <!DOCTYPE html> <ul> <li> <div style="overflow:hidden; height:100px; background:hotpink;"></div> <div> ←Here's the first line. Should the marker be here? </div> </li> </ul> Therefore I feel that we don't want that code, since omitting it will make the behavior more deterministic than without it, with less code.
https://codereview.chromium.org/2759883002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp (right): https://codereview.chromium.org/2759883002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp:890: toLayoutListItem(this)->addLogicalHeightFromChild(child); On 2017/03/20 13:51:20, mstensho wrote: > On 2017/03/20 12:56:44, cathiechentx wrote: > > On 2017/03/20 10:19:01, mstensho wrote: > > > Without this special-code, the list item marker would occupy a line on its > > own, > > > and here's how you attempt to avoid it. But I'm not sure if you really have > to > > > do that. Edge (but not Firefox) renders your test just like that, i.e. with > a > > > marker on a separate line, followed by the overflow:hidden block. > > > > > > So I suggest you do that too (i.e just back out this change), unless there's > a > > > spec that requires us to behave differently. If we can keep it simple, we > > should > > > do that. :) > > > > Yeah, that make sense. But I'm concerned about the difference between > > overflow:hidden and overflow:visible might confuse people. With > overflow:hidden > > child we change it from no list marker to occupied one-line list marker. It > > doesn't seem to have improved. > > Looks like Firefox doesn't really put the marker next to the line box - it just > fakes it somehow: > <!DOCTYPE html> > <ul> > <li> > <div style="position:relative; top:100px;"> > ←Marker to the left here? > </div> > </li> > </ul> > > In Firefox, the list item marker stays put, while the DIV and the line of text > are moved down by 100px. Edge and Blink render it identically, with the marker > next to the DIV. > > The new code inside LayoutListItem (invoked from here) tries to make it appear > as if the marker is part of the line inside the DIV by kind of taking the marker > out of flow. It falls apart as soon as one does something like this, though: > <!DOCTYPE html> > <ul> > <li> > <div style="overflow:hidden; margin-top:100px;"> > ←Is the marker still there? > </div> > </li> > </ul> > > Or this: > <!DOCTYPE html> > <ul> > <li> > <div style="overflow:hidden;"> > ←Marker baseline alignment > <span style="font-size:5em;">FAIL</span> > </div> > </li> > </ul> > > Here the marker aligns itself with an imaginary line inside the hotpink block: > <!DOCTYPE html> > <ul> > <li> > <div style="overflow:hidden; height:100px; background:hotpink;"></div> > <div> > ←Here's the first line. Should the marker be here? > </div> > </li> > </ul> > > Therefore I feel that we don't want that code, since omitting it will make the > behavior more deterministic than without it, with less code. Thanks very much for the patient explanation! This is much more complicate than I imagine. I'm trying to figure it out in this way: 1. Directly add marker into list item 2. Skip marker while list item layout children 3. In positionListMarker() find the first leaf child and calculate its offset from list item. 4. Move marker by this offset This approach is not simple at all and may not cover other issues. List item is very steady now, maybe I shouldn't make this change.
https://codereview.chromium.org/2759883002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp (right): https://codereview.chromium.org/2759883002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp:890: toLayoutListItem(this)->addLogicalHeightFromChild(child); On 2017/03/21 14:07:08, cathiechentx wrote: > On 2017/03/20 13:51:20, mstensho wrote: > > On 2017/03/20 12:56:44, cathiechentx wrote: > > > On 2017/03/20 10:19:01, mstensho wrote: > > > > Without this special-code, the list item marker would occupy a line on its > > > own, > > > > and here's how you attempt to avoid it. But I'm not sure if you really > have > > to > > > > do that. Edge (but not Firefox) renders your test just like that, i.e. > with > > a > > > > marker on a separate line, followed by the overflow:hidden block. > > > > > > > > So I suggest you do that too (i.e just back out this change), unless > there's > > a > > > > spec that requires us to behave differently. If we can keep it simple, we > > > should > > > > do that. :) > > > > > > Yeah, that make sense. But I'm concerned about the difference between > > > overflow:hidden and overflow:visible might confuse people. With > > overflow:hidden > > > child we change it from no list marker to occupied one-line list marker. It > > > doesn't seem to have improved. > > > > Looks like Firefox doesn't really put the marker next to the line box - it > just > > fakes it somehow: > > <!DOCTYPE html> > > <ul> > > <li> > > <div style="position:relative; top:100px;"> > > ←Marker to the left here? > > </div> > > </li> > > </ul> > > > > In Firefox, the list item marker stays put, while the DIV and the line of text > > are moved down by 100px. Edge and Blink render it identically, with the marker > > next to the DIV. > > > > The new code inside LayoutListItem (invoked from here) tries to make it appear > > as if the marker is part of the line inside the DIV by kind of taking the > marker > > out of flow. It falls apart as soon as one does something like this, though: > > <!DOCTYPE html> > > <ul> > > <li> > > <div style="overflow:hidden; margin-top:100px;"> > > ←Is the marker still there? > > </div> > > </li> > > </ul> > > > > Or this: > > <!DOCTYPE html> > > <ul> > > <li> > > <div style="overflow:hidden;"> > > ←Marker baseline alignment > > <span style="font-size:5em;">FAIL</span> > > </div> > > </li> > > </ul> > > > > Here the marker aligns itself with an imaginary line inside the hotpink block: > > <!DOCTYPE html> > > <ul> > > <li> > > <div style="overflow:hidden; height:100px; background:hotpink;"></div> > > <div> > > ←Here's the first line. Should the marker be here? > > </div> > > </li> > > </ul> > > > > Therefore I feel that we don't want that code, since omitting it will make the > > behavior more deterministic than without it, with less code. > > Thanks very much for the patient explanation! This is much more complicate than > I imagine. I'm trying to figure it out in this way: > > 1. Directly add marker into list item > 2. Skip marker while list item layout children > 3. In positionListMarker() find the first leaf child and calculate its offset > from list item. > 4. Move marker by this offset > > This approach is not simple at all and may not cover other issues. List item is > very steady now, maybe I shouldn't make this change. Yes, I think you should omit the addLogicalHeightFromChild() part of your change.
Thanks for reply:) There's one more thing need to be clarified: Should I omit this test case too? It's quirk to write a reftest with marker occupied one line. And we might find a simple way to put marker on the left side of content in the future. https://codereview.chromium.org/2759883002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp (right): https://codereview.chromium.org/2759883002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp:890: toLayoutListItem(this)->addLogicalHeightFromChild(child); On 2017/03/22 22:28:55, mstensho wrote: > On 2017/03/21 14:07:08, cathiechentx wrote: > > On 2017/03/20 13:51:20, mstensho wrote: > > > On 2017/03/20 12:56:44, cathiechentx wrote: > > > > On 2017/03/20 10:19:01, mstensho wrote: > > > > > Without this special-code, the list item marker would occupy a line on > its > > > > own, > > > > > and here's how you attempt to avoid it. But I'm not sure if you really > > have > > > to > > > > > do that. Edge (but not Firefox) renders your test just like that, i.e. > > with > > > a > > > > > marker on a separate line, followed by the overflow:hidden block. > > > > > > > > > > So I suggest you do that too (i.e just back out this change), unless > > there's > > > a > > > > > spec that requires us to behave differently. If we can keep it simple, > we > > > > should > > > > > do that. :) > > > > > > > > Yeah, that make sense. But I'm concerned about the difference between > > > > overflow:hidden and overflow:visible might confuse people. With > > > overflow:hidden > > > > child we change it from no list marker to occupied one-line list marker. > It > > > > doesn't seem to have improved. > > > > > > Looks like Firefox doesn't really put the marker next to the line box - it > > just > > > fakes it somehow: > > > <!DOCTYPE html> > > > <ul> > > > <li> > > > <div style="position:relative; top:100px;"> > > > ←Marker to the left here? > > > </div> > > > </li> > > > </ul> > > > > > > In Firefox, the list item marker stays put, while the DIV and the line of > text > > > are moved down by 100px. Edge and Blink render it identically, with the > marker > > > next to the DIV. > > > > > > The new code inside LayoutListItem (invoked from here) tries to make it > appear > > > as if the marker is part of the line inside the DIV by kind of taking the > > marker > > > out of flow. It falls apart as soon as one does something like this, though: > > > <!DOCTYPE html> > > > <ul> > > > <li> > > > <div style="overflow:hidden; margin-top:100px;"> > > > ←Is the marker still there? > > > </div> > > > </li> > > > </ul> > > > > > > Or this: > > > <!DOCTYPE html> > > > <ul> > > > <li> > > > <div style="overflow:hidden;"> > > > ←Marker baseline alignment > > > <span style="font-size:5em;">FAIL</span> > > > </div> > > > </li> > > > </ul> > > > > > > Here the marker aligns itself with an imaginary line inside the hotpink > block: > > > <!DOCTYPE html> > > > <ul> > > > <li> > > > <div style="overflow:hidden; height:100px; > background:hotpink;"></div> > > > <div> > > > ←Here's the first line. Should the marker be here? > > > </div> > > > </li> > > > </ul> > > > > > > Therefore I feel that we don't want that code, since omitting it will make > the > > > behavior more deterministic than without it, with less code. > > > > Thanks very much for the patient explanation! This is much more complicate > than > > I imagine. I'm trying to figure it out in this way: > > > > 1. Directly add marker into list item > > 2. Skip marker while list item layout children > > 3. In positionListMarker() find the first leaf child and calculate its offset > > from list item. > > 4. Move marker by this offset > > > > This approach is not simple at all and may not cover other issues. List item > is > > very steady now, maybe I shouldn't make this change. > > Yes, I think you should omit the addLogicalHeightFromChild() part of your > change. OK, that will be done.
On 2017/03/23 10:13:27, cathiechentx wrote: > Thanks for reply:) There's one more thing need to be clarified: > > Should I omit this test case too? It's quirk to write a reftest with marker > occupied one line. And we might find a simple way to put marker on the left side > of content in the future. > We definitely want a test for this. It shouldn't be too hard to write a ref. Isn't the following going to work? <li> <br> xxx </li>
Description was changed from ========== Get markers back while list items have overflow:hidden children 1. Add marker into list item not overflow:hidden child while construct layout tree. 2. In order to stay consistent with common list item which position its marker on the left, do not let marker effect the height of list item while it's layouting block children. BUG=626293 ========== to ========== Get markers back while list items have overflow:hidden children Add marker into list item not overflow:hidden child while construct layout tree. BUG=626293 ==========
On 2017/03/23 10:18:03, mstensho wrote: > > We definitely want a test for this. It shouldn't be too hard to write a ref. > Isn't the following going to work? > > <li> > <br> > xxx > </li> Yeah, it works. Thanks:)
On 2017/03/23 10:18:03, mstensho wrote: > > We definitely want a test for this. It shouldn't be too hard to write a ref. > Isn't the following going to work? > > <li> > <br> > xxx > </li> Yeah, it works. Thanks:)
The CQ bit was checked by mstensho@opera.com
lgtm
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
The author cathiechen@tencent.com has not signed Google Contributor License Agreement. Please visit https://cla.developers.google.com to sign and manage CLA.
The CQ bit was checked by mstensho@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
The author cathiechen@tencent.com has not signed Google Contributor License Agreement. Please visit https://cla.developers.google.com to sign and manage CLA.
On 2017/03/23 18:46:59, commit-bot: I haz the power wrote: > The author mailto:cathiechen@tencent.com has not signed Google Contributor License > Agreement. Please visit https://cla.developers.google.com to sign and manage > CLA. @cathiechen: I see that you have landed several CLs before, so I don't know what's going on here. https://codereview.chromium.org/2270683002#msg5 claims that your CLA was in order, although there was an issue landing that CL too: https://codereview.chromium.org/2270683002#msg19
On 2017/03/23 at 18:53:16, mstensho wrote: > On 2017/03/23 18:46:59, commit-bot: I haz the power wrote: > > The author mailto:cathiechen@tencent.com has not signed Google Contributor License > > Agreement. Please visit https://cla.developers.google.com to sign and manage > > CLA. > > @cathiechen: I see that you have landed several CLs before, so I don't know what's going on here. https://codereview.chromium.org/2270683002#msg5 claims that your CLA was in order, although there was an issue landing that CL too: https://codereview.chromium.org/2270683002#msg19 cathiechen, can you please file a bug for this? Please file under Infra>CQ and assign to benhenry.
On 2017/03/23 19:09:09, pdr. wrote: > On 2017/03/23 at 18:53:16, mstensho wrote: > > On 2017/03/23 18:46:59, commit-bot: I haz the power wrote: > > > The author mailto:cathiechen@tencent.com has not signed Google Contributor > License > > > Agreement. Please visit https://cla.developers.google.com to sign and manage > > > CLA. > > > > @cathiechen: I see that you have landed several CLs before, so I don't know > what's going on here. https://codereview.chromium.org/2270683002#msg5 claims > that your CLA was in order, although there was an issue landing that CL too: > https://codereview.chromium.org/2270683002#msg19 > > cathiechen, can you please file a bug for this? Please file under Infra>CQ and > assign to benhenry. Or re-open crbug.com/640663 ?
The CQ bit was checked by cathiechen@tencent.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/03/23 19:13:46, mstensho wrote: > On 2017/03/23 19:09:09, pdr. wrote: > > On 2017/03/23 at 18:53:16, mstensho wrote: > > > On 2017/03/23 18:46:59, commit-bot: I haz the power wrote: > > > > The author mailto:cathiechen@tencent.com has not signed Google Contributor > > License > > > > Agreement. Please visit https://cla.developers.google.com to sign and > manage > > > > CLA. > > > > > > @cathiechen: I see that you have landed several CLs before, so I don't know > > what's going on here. https://codereview.chromium.org/2270683002#msg5 claims > > that your CLA was in order, although there was an issue landing that CL too: > > https://codereview.chromium.org/2270683002#msg19 > > > > cathiechen, can you please file a bug for this? Please file under Infra>CQ and > > assign to benhenry. > > Or re-open crbug.com/640663 ? It must have something to do with my account binding actions before(binding and unbinding it to another account...). Somehow I quit the contributor group whose CLA I used. I have rejoined that group. It could work again. Sorry for the mess! And thanks again! pdr@ mstensho@
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by cathiechen@tencent.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 60001, "attempt_start_ts": 1490332733889760, "parent_rev": "026cead1b2ff13a78bc044c62efd63b8efb0041e", "commit_rev": "fd1e60f0bee4b6b25d898f4f4f859aa38d1a9643"}
Message was sent while issue was closed.
Description was changed from ========== Get markers back while list items have overflow:hidden children Add marker into list item not overflow:hidden child while construct layout tree. BUG=626293 ========== to ========== Get markers back while list items have overflow:hidden children Add marker into list item not overflow:hidden child while construct layout tree. BUG=626293 Review-Url: https://codereview.chromium.org/2759883002 Cr-Commit-Position: refs/heads/master@{#459372} Committed: https://chromium.googlesource.com/chromium/src/+/fd1e60f0bee4b6b25d898f4f4f85... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/fd1e60f0bee4b6b25d898f4f4f85... |