Pass CSSParserLocalContext down to CSSMathExpressionNodeParser [chromium/src : main]
Munira Tursunova (Gerrit)
Munira Tursunova added 6 comments![]( "Open in Gerrit")
Munira TursunovaIt's generally hard for me (and you later on) to tell whether the default constructions (without a property) of CSSParserLocalContext are intentional or an oversight.
I think we should disallow the default construction and require that we pass some argument that annotates why we can ignore the property. Otherwise it's hard to tell when you're done.
I can disallow the default constructor and pass a pointer to CSSPropertyName to the constructor(so we would pass nullptr if there is no property specified, for instance media queries case), what do you think?
Pass CSSParserContext down to CSSMathExpressionNodeParserMunira TursunovaCSSParserLocalContext?
Done
This CL does not include passing CSSParser context with relevantMunira Tursunova"the relevant"
Done
This CL does not include passing CSSParser context with relevantMunira Tursunova"pass a CSSParserLocalContext"
Done
property name in case of at rule descriptors and substitution functions,Munira Tursunova"for"
Done
CSSParserLocalContext local_context_2 = CSSParserLocalContext();Rune LillesveenThis is a workaround, since using the argument CSSParserLocalContext will cause `animations/interpolation/webkit-clip-path-interpolation.html` test to fail, see https://ci.chromium.org/ui/p/chromium/builders/try/linux-rel/2504640/overview. I'm not sure if the test is correct, I think we should be using alias parsing here, but using local_context here will make alias parsing for css animation, but not css transitions, needs more investigation.
Munira TursunovaCould you elaborate?
In what way is this related to transitions and animations?
What do you mean by alias parsing for transitions? Transitions work on computed values of the standardized property since this is a pure alias, right?
Before we used default constructor for local_context here. Then this value is passed to `ConsumeRadii` which checks `local_context.UseAliasParsing()` for legacy syntax pasrsing, which for default constructor will always return false.
This causes `animations/interpolation/webkit-clip-path-interpolation.html` test to fail, see https://ci.chromium.org/ui/p/chromium/builders/try/linux-rel/2504640/overview. Please check the test expectations there to better understand the problem. If I just use local_context that uses `CSSParserLocalContext` with the property, so `local_context.UseAliasParsing()` will return true for --webkit properties (failing tests uses -webkit-clip-path property). I will get errors like this:
```
[FAIL] CSS Transitions: property <-webkit-clip-path> from [inset(1px 2px round 100px 200px)] to [inset(101px 102px 101px 102px)] at (-0.3) is [inset(-29px -28px round 130px 260px)]
assert_equals: expected "inset ( - 29px - 28px round 130px / 260px ) " but got "inset ( - 29px - 28px round 130px 260px ) "
[FAIL] CSS Transitions with transition: all: property <-webkit-clip-path> from [inset(1px 2px round 100px 200px)] to [inset(101px 102px 101px 102px)] at (-0.3) is [inset(-29px -28px round 130px 260px)]
assert_equals: expected "inset ( - 29px - 28px round 130px / 260px ) " but got "inset ( - 29px - 28px round 130px 260px ) "
```
I don't know exactly why this happens only for transitions, that's why I added a workaround to use a default constructor here, so that we get the same behaviour as before this change. I can fix it later, not to expand this CL even further.
Related details
- Rune Lillesveen
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Munira Tursunova (Gerrit)
Munira Tursunova added 2 comments![]( "Open in Gerrit")
CSSParserLocalContext local_context_2 = CSSParserLocalContext();Munira Tursunovanit: Variable name 'local_context_2' contains a number. Since the 'CSSParserLocalContext&' parameter is unnamed, consider naming this variable 'local_context'. (Blink Style Guide: Naming)
To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **OK But Won't Fix**: reason | **Later**: b/<bug_id> | **Invalid:** reason
_This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent)._
_AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve._
_[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
Done
CSSParserLocalContext& local_context) const {Munira TursunovaParameter 'local_context' appears to be unused. Consider omitting the name to avoid unused parameter warnings.
To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **OK But Won't Fix**: reason | **Later**: b/<bug_id> | **Invalid:** reason
_This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent)._
_AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve._
_[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
Done
Related details
- Rune Lillesveen
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Rune Lillesveen (Gerrit)
Rune Lillesveen added 3 comments![]( "Open in Gerrit")
Munira TursunovaIt's generally hard for me (and you later on) to tell whether the default constructions (without a property) of CSSParserLocalContext are intentional or an oversight.
I think we should disallow the default construction and require that we pass some argument that annotates why we can ignore the property. Otherwise it's hard to tell when you're done.
I can disallow the default constructor and pass a pointer to CSSPropertyName to the constructor(so we would pass nullptr if there is no property specified, for instance media queries case), what do you think?
I can disallow the default constructor and pass a pointer to CSSPropertyName to the constructor(so we would pass nullptr if there is no property specified, for instance media queries case), what do you think?
That is basically the same as the default constructor. I want to make sure for every case we're not passing a property that I can immediately read from the construction whether it is done intentionally, whether it is covered by the spec or needs a spec resolution, or if it should be a TODO because we haven't propagated the property where we need it.
It's currently super-hard for me to review what the case is for these default constructor instances, and unless you document your intent, it's impossible for anyone coming back to this code later to understand what your intent was.
Take typed OM as an example. If it's OK to pass a CSSParserLocalContext without a property when parsing typed OM values, make a constructor or a function for constructing that context that you use commonly for all such invocations, and document that constructor/function in a way that it's clear that it's intentional that we don't pass a property, and that we will populate the random() function values with the correct property when the value is set for an actual property (assuming that's how it's supposed to work).
CSSParserLocalContext local_context_2 = CSSParserLocalContext();nit: Variable name 'local_context_2' contains a number. Since the 'CSSParserLocalContext&' parameter is unnamed, consider naming this variable 'local_context'. (Blink Style Guide: Naming)
To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **OK But Won't Fix**: reason | **Later**: b/<bug_id> | **Invalid:** reason
_This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent)._
_AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve._
_[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
+1
CSSParserLocalContext local_context_2 = CSSParserLocalContext();Rune LillesveenThis is a workaround, since using the argument CSSParserLocalContext will cause `animations/interpolation/webkit-clip-path-interpolation.html` test to fail, see https://ci.chromium.org/ui/p/chromium/builders/try/linux-rel/2504640/overview. I'm not sure if the test is correct, I think we should be using alias parsing here, but using local_context here will make alias parsing for css animation, but not css transitions, needs more investigation.
Munira TursunovaCould you elaborate?
In what way is this related to transitions and animations?
What do you mean by alias parsing for transitions? Transitions work on computed values of the standardized property since this is a pure alias, right?
Before we used default constructor for local_context here. Then this value is passed to `ConsumeRadii` which checks `local_context.UseAliasParsing()` for legacy syntax pasrsing, which for default constructor will always return false.
This causes `animations/interpolation/webkit-clip-path-interpolation.html` test to fail, see https://ci.chromium.org/ui/p/chromium/builders/try/linux-rel/2504640/overview. Please check the test expectations there to better understand the problem. If I just use local_context that uses `CSSParserLocalContext` with the property, so `local_context.UseAliasParsing()` will return true for --webkit properties (failing tests uses -webkit-clip-path property). I will get errors like this:
```
[FAIL] CSS Transitions: property <-webkit-clip-path> from [inset(1px 2px round 100px 200px)] to [inset(101px 102px 101px 102px)] at (-0.3) is [inset(-29px -28px round 130px 260px)]
assert_equals: expected "inset ( - 29px - 28px round 130px / 260px ) " but got "inset ( - 29px - 28px round 130px 260px ) "
[FAIL] CSS Transitions with transition: all: property <-webkit-clip-path> from [inset(1px 2px round 100px 200px)] to [inset(101px 102px 101px 102px)] at (-0.3) is [inset(-29px -28px round 130px 260px)]
assert_equals: expected "inset ( - 29px - 28px round 130px / 260px ) " but got "inset ( - 29px - 28px round 130px 260px ) "
```I don't know exactly why this happens only for transitions, that's why I added a workaround to use a default constructor here, so that we get the same behaviour as before this change. I can fix it later, not to expand this CL even further.
This causes `animations/interpolation/webkit-clip-path-interpolation.html` test to fail, see https://ci.chromium.org/ui/p/chromium/builders/try/linux-rel/2504640/overview. Please check the test expectations there to better understand the problem.
Yes, I did see that. I wanted you to explain it to me so I didn't have to dive into the problem :-)
I don't know exactly why this happens only for transitions, that's why I added a workaround to use a default constructor here, so that we get the same behaviour as before this change. I can fix it later, not to expand this CL even further.
- Munira Tursunova
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Munira Tursunova (Gerrit)
Munira Tursunova added 5 comments![]( "Open in Gerrit")
Munira TursunovaWould it be possible to split this into smaller CLs?
Munira TursunovaI don't think so, there is not much logical changes here, all the logical changes are in `third_party/blink/renderer/core/css/properties/css_parsing_utils.cc`, the rest is mostly creating blank CSSParserContext and passing it as an new argument to the functions, so that it would compile.
Done
Munira TursunovaIt's generally hard for me (and you later on) to tell whether the default constructions (without a property) of CSSParserLocalContext are intentional or an oversight.
I think we should disallow the default construction and require that we pass some argument that annotates why we can ignore the property. Otherwise it's hard to tell when you're done.
Rune LillesveenI can disallow the default constructor and pass a pointer to CSSPropertyName to the constructor(so we would pass nullptr if there is no property specified, for instance media queries case), what do you think?
I can disallow the default constructor and pass a pointer to CSSPropertyName to the constructor(so we would pass nullptr if there is no property specified, for instance media queries case), what do you think?
That is basically the same as the default constructor. I want to make sure for every case we're not passing a property that I can immediately read from the construction whether it is done intentionally, whether it is covered by the spec or needs a spec resolution, or if it should be a TODO because we haven't propagated the property where we need it.
It's currently super-hard for me to review what the case is for these default constructor instances, and unless you document your intent, it's impossible for anyone coming back to this code later to understand what your intent was.
Take typed OM as an example. If it's OK to pass a CSSParserLocalContext without a property when parsing typed OM values, make a constructor or a function for constructing that context that you use commonly for all such invocations, and document that constructor/function in a way that it's clear that it's intentional that we don't pass a property, and that we will populate the random() function values with the correct property when the value is set for an actual property (assuming that's how it's supposed to work).
Added separate constructors for groups of cases without property names as we discussed offline.
CSSParserLocalContext local_context = CSSParserLocalContext();Munira TursunovaWhy the assignment here? Why not just `CSSParserLocalContext local_context;`?
(Goes for all similar declarations)
But also see the general comment about default construction.
Done
CSSParserLocalContext local_context_2 = CSSParserLocalContext();Rune Lillesveennit: Variable name 'local_context_2' contains a number. Since the 'CSSParserLocalContext&' parameter is unnamed, consider naming this variable 'local_context'. (Blink Style Guide: Naming)
To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **OK But Won't Fix**: reason | **Later**: b/<bug_id> | **Invalid:** reason
_This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent)._
_AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve._
_[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
+1
Done already?
CSSParserLocalContext local_context_2 = CSSParserLocalContext();Rune LillesveenThis is a workaround, since using the argument CSSParserLocalContext will cause `animations/interpolation/webkit-clip-path-interpolation.html` test to fail, see https://ci.chromium.org/ui/p/chromium/builders/try/linux-rel/2504640/overview. I'm not sure if the test is correct, I think we should be using alias parsing here, but using local_context here will make alias parsing for css animation, but not css transitions, needs more investigation.
Munira TursunovaCould you elaborate?
In what way is this related to transitions and animations?
What do you mean by alias parsing for transitions? Transitions work on computed values of the standardized property since this is a pure alias, right?
Rune LillesveenBefore we used default constructor for local_context here. Then this value is passed to `ConsumeRadii` which checks `local_context.UseAliasParsing()` for legacy syntax pasrsing, which for default constructor will always return false.
This causes `animations/interpolation/webkit-clip-path-interpolation.html` test to fail, see https://ci.chromium.org/ui/p/chromium/builders/try/linux-rel/2504640/overview. Please check the test expectations there to better understand the problem. If I just use local_context that uses `CSSParserLocalContext` with the property, so `local_context.UseAliasParsing()` will return true for --webkit properties (failing tests uses -webkit-clip-path property). I will get errors like this:
```
[FAIL] CSS Transitions: property <-webkit-clip-path> from [inset(1px 2px round 100px 200px)] to [inset(101px 102px 101px 102px)] at (-0.3) is [inset(-29px -28px round 130px 260px)]
assert_equals: expected "inset ( - 29px - 28px round 130px / 260px ) " but got "inset ( - 29px - 28px round 130px 260px ) "
[FAIL] CSS Transitions with transition: all: property <-webkit-clip-path> from [inset(1px 2px round 100px 200px)] to [inset(101px 102px 101px 102px)] at (-0.3) is [inset(-29px -28px round 130px 260px)]
assert_equals: expected "inset ( - 29px - 28px round 130px / 260px ) " but got "inset ( - 29px - 28px round 130px 260px ) "
```I don't know exactly why this happens only for transitions, that's why I added a workaround to use a default constructor here, so that we get the same behaviour as before this change. I can fix it later, not to expand this CL even further.
This causes `animations/interpolation/webkit-clip-path-interpolation.html` test to fail, see https://ci.chromium.org/ui/p/chromium/builders/try/linux-rel/2504640/overview. Please check the test expectations there to better understand the problem.
Yes, I did see that. I wanted you to explain it to me so I didn't have to dive into the problem :-)
I don't know exactly why this happens only for transitions, that's why I added a workaround to use a default constructor here, so that we get the same behaviour as before this change. I can fix it later, not to expand this CL even further.
Could you leave a TODO then?
- Rune Lillesveen
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Rune Lillesveen (Gerrit)
Rune Lillesveen added 1 comment![]( "Open in Gerrit")
static CSSParserLocalContext CreateWithoutPropertyForCSSOM() {Now, each of these need documentation.
For the cases where we clearly don't need a property covered by the spec, say why a property is not relevant.
For the cases that needs fixing, or needs spec clarification, please add TODOs with an explanation.
Related details
- Munira Tursunova
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Munira Tursunova (Gerrit)
Munira Tursunova added 2 comments![]( "Open in Gerrit")
static CSSParserLocalContext CreateWithoutPropertyForCSSOM() {Now, each of these need documentation.
For the cases where we clearly don't need a property covered by the spec, say why a property is not relevant.
For the cases that needs fixing, or needs spec clarification, please add TODOs with an explanation.
Done
CSSParserLocalContext local_context_2 = CSSParserLocalContext();Rune Lillesveennit: Variable name 'local_context_2' contains a number. Since the 'CSSParserLocalContext&' parameter is unnamed, consider naming this variable 'local_context'. (Blink Style Guide: Naming)
To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **OK But Won't Fix**: reason | **Later**: b/<bug_id> | **Invalid:** reason
_This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent)._
_AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve._
_[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
Munira Tursunova+1
Done already?
- Rune Lillesveen
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Rune Lillesveen (Gerrit)
Rune Lillesveen added 6 comments![]( "Open in Gerrit")
// For non custom properties, need to pass CSSPropertyName with unresolvedDo you mean "standard properties", or "non-registered custom properties"?
When parsing non-registered custom properties, we only treat values as tokens without meaning, so we will never recognize random() functions as such. If this is what it's for, write something along those lines.
// TODO(crbug.com/413385732): We used this constructor to create a localuse
// property-dependent random() values.We might want to disallow random() at parse time for setting values on the canvas contexts. Should be a canvas spec issue.
// TODO(crbug.com/413385732) This function is only used in MathML to parse
// math length, figure out if we have property info there that we can passThis is for MathML attributes, not css properties, right?
// There is a chance we can have random() insideident() in there, but it's notMissing space
// Should be only used for testing."only be"
Related details
- Munira Tursunova
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Munira Tursunova (Gerrit)
Munira Tursunova added 6 comments![]( "Open in Gerrit")
// For non custom properties, need to pass CSSPropertyName with unresolvedDo you mean "standard properties", or "non-registered custom properties"?
When parsing non-registered custom properties, we only treat values as tokens without meaning, so we will never recognize random() functions as such. If this is what it's for, write something along those lines.
>Do you mean "standard properties", or "non-registered custom properties"?
"standard properties", I forgot the correct term 😄
>When parsing non-registered custom properties, we only treat values as tokens without meaning, so we will never recognize random() functions as such.
For random() inside non registered custom properties, I'm planning to handle them separately during style cascade, not implemented yet.
// TODO(crbug.com/413385732): We used this constructor to create a localMunira Tursunovause
Done
// property-dependent random() values.We might want to disallow random() at parse time for setting values on the canvas contexts. Should be a canvas spec issue.
Maybe I can do this in another CL? This one is already quite big. I left TODO linking the random() bug for now, and will disallow them in separate CL, wdyt?
// TODO(crbug.com/413385732) This function is only used in MathML to parse
// math length, figure out if we have property info there that we can passThis is for MathML attributes, not css properties, right?
for attributes, yes. Though I thought we cen better use attribute name in that case, I removed this constructor and using the one with attribute name. Since this behaviour is not specced I opened a new bug and left todo in case if we want to change it in the future.
// There is a chance we can have random() insideident() in there, but it's notMunira TursunovaMissing space
Done
// Should be only used for testing.Munira Tursunova"only be"
Done
Related details
- Rune Lillesveen
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Rune Lillesveen (Gerrit)
Rune Lillesveen voted and added 3 comments![]( "Open in Gerrit")
Votes added by Rune Lillesveen
| Code-Review | +1 |
3 comments
// property-dependent random() values.Munira TursunovaWe might want to disallow random() at parse time for setting values on the canvas contexts. Should be a canvas spec issue.
Maybe I can do this in another CL? This one is already quite big. I left TODO linking the random() bug for now, and will disallow them in separate CL, wdyt?
Yeah, I didn't mean you should write the code for that now. I thought it could make sense to mention that in the comment.
// TODO(crbug.com/413385732) This function is only used in MathML to parse
// math length, figure out if we have property info there that we can passMunira TursunovaThis is for MathML attributes, not css properties, right?
for attributes, yes. Though I thought we cen better use attribute name in that case, I removed this constructor and using the one with attribute name. Since this behaviour is not specced I opened a new bug and left todo in case if we want to change it in the future.
That might be, but needs to be raised as a spec issue if it's not clear from the spec(s).
Related details
- Munira Tursunova
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Munira Tursunova (Gerrit)
Munira Tursunova voted and added 2 comments![]( "Open in Gerrit")
Votes added by Munira Tursunova
| Commit-Queue | +2 |
2 comments
// property-dependent random() values.Munira TursunovaWe might want to disallow random() at parse time for setting values on the canvas contexts. Should be a canvas spec issue.
Rune LillesveenMaybe I can do this in another CL? This one is already quite big. I left TODO linking the random() bug for now, and will disallow them in separate CL, wdyt?
Yeah, I didn't mean you should write the code for that now. I thought it could make sense to mention that in the comment.
added to the comment
// TODO(crbug.com/413385732) This function is only used in MathML to parse
// math length, figure out if we have property info there that we can passMunira TursunovaThis is for MathML attributes, not css properties, right?
Rune Lillesveenfor attributes, yes. Though I thought we cen better use attribute name in that case, I removed this constructor and using the one with attribute name. Since this behaviour is not specced I opened a new bug and left todo in case if we want to change it in the future.
That might be, but needs to be raised as a spec issue if it's not clear from the spec(s).
👍 will do that together with random inside attr()
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
8 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: third_party/blink/renderer/core/css/parser/css_parser_local_context.h
Insertions: 2, Deletions: 1.
The diff is too large to show. Please review the diff.
```
```
The name of the file: third_party/blink/renderer/core/css/properties/css_parsing_utils.h
Insertions: 1, Deletions: 1.
The diff is too large to show. Please review the diff.
```
```
The name of the file: third_party/blink/renderer/core/inspector/inspector_css_agent.cc
Insertions: 5, Deletions: 2.
The diff is too large to show. Please review the diff.
```
```
The name of the file: third_party/blink/renderer/core/css/properties/css_parsing_utils.cc
Insertions: 0, Deletions: 16.
The diff is too large to show. Please review the diff.
```
```
The name of the file: third_party/blink/renderer/core/css/properties/longhands/longhands_custom.cc
Insertions: 2, Deletions: 2.
The diff is too large to show. Please review the diff.
```
Change information
Pass CSSParserLocalContext down to CSSMathExpressionNodeParser
This CL does not pass a CSSParserLocalContext with the relevant property
name for at rule descriptors and substitution functions, left TODOs for
them.
- M third_party/blink/renderer/core/animation/effect_input.cc
- M third_party/blink/renderer/core/animation/interpolable_value_test.cc
- M third_party/blink/renderer/core/animation/timeline_offset.cc
- M third_party/blink/renderer/core/animation/timeline_trigger_range.cc
- M third_party/blink/renderer/core/css/css_attr_type.cc
- M third_party/blink/renderer/core/css/css_attr_type.h
- M third_party/blink/renderer/core/css/css_attr_type_test.cc
- M third_party/blink/renderer/core/css/css_math_expression_node.cc
- M third_party/blink/renderer/core/css/css_math_expression_node.h
- M third_party/blink/renderer/core/css/css_math_expression_node_test.cc
- M third_party/blink/renderer/core/css/css_syntax_definition.cc
- M third_party/blink/renderer/core/css/css_test_helpers.cc
- M third_party/blink/renderer/core/css/cssom/css_color_value.cc
- M third_party/blink/renderer/core/css/cssom/css_numeric_value.cc
- M third_party/blink/renderer/core/css/media_query_exp.cc
- M third_party/blink/renderer/core/css/parser/at_rule_counter_style_descriptor_parser.cc
- M third_party/blink/renderer/core/css/parser/at_rule_descriptor_parser.cc
- M third_party/blink/renderer/core/css/parser/at_rule_font_palette_values_descriptor_parser.cc
- M third_party/blink/renderer/core/css/parser/css_parser.cc
- M third_party/blink/renderer/core/css/parser/css_parser.h
- M third_party/blink/renderer/core/css/parser/css_parser_impl.cc
- M third_party/blink/renderer/core/css/parser/css_parser_local_context.h
- M third_party/blink/renderer/core/css/parser/css_property_parser.cc
- M third_party/blink/renderer/core/css/parser/css_selector_parser.cc
- M third_party/blink/renderer/core/css/parser/css_variable_parser.cc
- M third_party/blink/renderer/core/css/parser/font_variant_alternates_parser.cc
- M third_party/blink/renderer/core/css/parser/font_variant_alternates_parser.h
- M third_party/blink/renderer/core/css/properties/css_color_function_parser.cc
- M third_party/blink/renderer/core/css/properties/css_color_function_parser.h
- M third_party/blink/renderer/core/css/properties/css_color_function_parser_test.cc
- 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/css_parsing_utils_test.cc
- M third_party/blink/renderer/core/css/properties/longhands/custom_property.cc
- M third_party/blink/renderer/core/css/properties/longhands/custom_property_test.cc
- M third_party/blink/renderer/core/css/properties/longhands/longhands_custom.cc
- M third_party/blink/renderer/core/css/properties/shorthands/shorthands_custom.cc
- M third_party/blink/renderer/core/css/resolver/style_cascade.cc
- M third_party/blink/renderer/core/css/resolver/style_resolver_test.cc
- M third_party/blink/renderer/core/inspector/inspector_css_agent.cc
- M third_party/blink/renderer/core/inspector/inspector_style_sheet.cc
- M third_party/blink/renderer/core/mathml/mathml_element.cc
- M third_party/blink/renderer/modules/canvas/canvas2d/canvas_style.cc
Code-Review: +1 by Rune Lillesveen
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |