[Gap Decorations]: Generalize `column-rule-color` parsing code [chromium/src : main]
0 views
Skip to first unread message
Sam Davis Omekara (Gerrit)
Nov 11, 2024, 7:57:43 PM11/11/24
to Alison Maher, Kevin Babbitt, Chromium LUCI CQ, Alexis Menard, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org
Attention needed from Alison Maher and Kevin Babbitt
Sam Davis Omekara voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Alison Maher
- Kevin Babbitt
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alison Maher (Gerrit)
Nov 12, 2024, 12:46:39 PM11/12/24
to Sam Davis Omekara, Kevin Babbitt, Chromium LUCI CQ, Alexis Menard, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org
Attention needed from Kevin Babbitt and Sam Davis Omekara
Alison Maher voted and added 2 comments![]( "Open in Gerrit")
Votes added by Alison Maher
| Code-Review | +1 |
2 comments
File third_party/blink/renderer/core/css/properties/css_parsing_utils.cc
Line 4983, Patchset 14 (Latest):
switch (property_type) {Alison Maher . resolved
Same comment as the other review to look into if we need this or can make this templatized and get the template value directly.
Line 4987, Patchset 14 (Latest):
}Alison Maher . unresolved
Might be worth adding a NOT_REACHED to the end to catch cases where a new type is supported, and we forget to add a case to one of the related switch statements
Related details
Attention is currently required from:
- Kevin Babbitt
- Sam Davis Omekara
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sam Davis Omekara (Gerrit)
Nov 12, 2024, 1:05:28 PM11/12/24
to Alison Maher, Kevin Babbitt, Chromium LUCI CQ, Alexis Menard, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org
Attention needed from Kevin Babbitt
Sam Davis Omekara added 1 comment![]( "Open in Gerrit")
File third_party/blink/renderer/core/css/properties/css_parsing_utils.cc
Alison Maher . resolved
Might be worth adding a NOT_REACHED to the end to catch cases where a new type is supported, and we forget to add a case to one of the related switch statements
Sam Davis Omekara
Added that at first, but from discussions with @kbab...@microsoft.com, he suggested I leave that to the compiler in this discussion:
https://chromium-review.googlesource.com/c/chromium/src/+/5962852/comment/40450188_14ee0066/
Related details
Attention is currently required from:
- Kevin Babbitt
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kevin Babbitt (Gerrit)
Nov 12, 2024, 1:49:18 PM11/12/24
to Sam Davis Omekara, Alison Maher, Chromium LUCI CQ, Alexis Menard, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org
Attention needed from Sam Davis Omekara
Kevin Babbitt voted and added 1 comment![]( "Open in Gerrit")
Votes added by Kevin Babbitt
| Code-Review | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Sam Davis Omekara
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sam Davis Omekara (Gerrit)
Nov 12, 2024, 1:49:47 PM11/12/24
to Kevin Babbitt, Alison Maher, Chromium LUCI CQ, Alexis Menard, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org
Sam Davis Omekara voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Nov 12, 2024, 2:05:59 PM11/12/24
to Sam Davis Omekara, Kevin Babbitt, Alison Maher, Alexis Menard, chromium...@chromium.org, apavlo...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[Gap Decorations]: Generalize `column-rule-color` parsing code
This CL generalizes the parsing logic for `column-rule-color` by
introducing the `ConsumeGapDecorationPropertyValue` function. This
function consumes different values based on the
`CSSGapDecorationPropertyType`. Following this, calls to `ConsumeColor`
are replaced with `ConsumeGapDecorationPropertyValue`. This approach
avoids the need to repeat the entire parsing logic when the only
difference is the value consumed.
Bug: 357648037
Change-Id: Ibfb47e9f1ad744d6df58917b70d576f79034ef65
Reviewed-by: Alison Maher <alm...@microsoft.com>
Commit-Queue: Sam Davis Omekara <samome...@microsoft.com>
Reviewed-by: Kevin Babbitt <kbab...@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#1381901}
Files:
- M third_party/blink/renderer/core/css/properties/css_parsing_utils.cc
- M third_party/blink/renderer/core/css/properties/css_parsing_utils.h
- M third_party/blink/renderer/core/css/properties/longhands/longhands_custom.cc
Change size: M
Delta: 3 files changed, 39 insertions(+), 23 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Kevin Babbitt, +1 by Alison Maher
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages