[hanging-punctuation] add parsing logic [chromium/src : main]
Lingqi Chi (Gerrit)
Lingqi Chi voted and added 2 comments![]( "Open in Gerrit")
Votes added by Lingqi Chi
| Commit-Queue | +1 |
2 comments
converter: "ConvertHangingPunctuation",I was using ConvertFlags<blink::HangingPunctuation>, but this template breaks StylePropertyMapTest.CSSKeywordValuesTest..
I wanted to fix the template by using `if ( To<CSSValueList>) {} else {}`, does it make sense to you?
Related details
- Koji Ishii
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. |
Koji Ishii (Gerrit)
Koji Ishii added 1 comment![]( "Open in Gerrit")
converter: "ConvertHangingPunctuation",I was using ConvertFlags<blink::HangingPunctuation>, but this template breaks StylePropertyMapTest.CSSKeywordValuesTest..
I wanted to fix the template by using `if ( To<CSSValueList>) {} else {}`, does it make sense to you?
I think it's better to crash (or DCHECK) when the code is not supposed to run in the real world. That said, using `ConvertFlags` looks better.
I'm not sure how the test fails, but is it possible to fix the test for this property?
Related details
- Lingqi Chi
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. |
Koji Ishii (Gerrit)
Koji Ishii added 1 comment![]( "Open in Gerrit")
converter: "ConvertHangingPunctuation",Koji IshiiI was using ConvertFlags<blink::HangingPunctuation>, but this template breaks StylePropertyMapTest.CSSKeywordValuesTest..
I wanted to fix the template by using `if ( To<CSSValueList>) {} else {}`, does it make sense to you?
I think it's better to crash (or DCHECK) when the code is not supposed to run in the real world. That said, using `ConvertFlags` looks better.
I'm not sure how the test fails, but is it possible to fix the test for this property?
Reading the comments in `CSSKeywordValuesTest`, since it says "DO NOT ADD ADDITIONAL PROPERTIES BELOW", maybe the change is missing some necessary code for Typed OM.
The blame told me that the test was added quite recently, 2025-11-18 by @futhark. You could ask him for advise.
Koji Ishii (Gerrit)
Koji Ishii added 1 comment![]( "Open in Gerrit")
converter: "ConvertHangingPunctuation",Koji IshiiI was using ConvertFlags<blink::HangingPunctuation>, but this template breaks StylePropertyMapTest.CSSKeywordValuesTest..
I wanted to fix the template by using `if ( To<CSSValueList>) {} else {}`, does it make sense to you?
Koji IshiiI think it's better to crash (or DCHECK) when the code is not supposed to run in the real world. That said, using `ConvertFlags` looks better.
I'm not sure how the test fails, but is it possible to fix the test for this property?
Reading the comments in `CSSKeywordValuesTest`, since it says "DO NOT ADD ADDITIONAL PROPERTIES BELOW", maybe the change is missing some necessary code for Typed OM.
The blame told me that the test was added quite recently, 2025-11-18 by @futhark. You could ask him for advise.
Since it's CSSOM, one of functions in https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/css/cssom/style_property_map.cc or https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/css/cssom/style_value_factory.cc is suspicious. Try setting break points and see if any of them hit?
Koji Ishii (Gerrit)
Koji Ishii added 1 comment![]( "Open in Gerrit")
converter: "ConvertHangingPunctuation",Koji IshiiI was using ConvertFlags<blink::HangingPunctuation>, but this template breaks StylePropertyMapTest.CSSKeywordValuesTest..
I wanted to fix the template by using `if ( To<CSSValueList>) {} else {}`, does it make sense to you?
Koji IshiiI think it's better to crash (or DCHECK) when the code is not supposed to run in the real world. That said, using `ConvertFlags` looks better.
I'm not sure how the test fails, but is it possible to fix the test for this property?
Koji IshiiReading the comments in `CSSKeywordValuesTest`, since it says "DO NOT ADD ADDITIONAL PROPERTIES BELOW", maybe the change is missing some necessary code for Typed OM.
The blame told me that the test was added quite recently, 2025-11-18 by @futhark. You could ask him for advise.
Since it's CSSOM, one of functions in https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/css/cssom/style_property_map.cc or https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/css/cssom/style_value_factory.cc is suspicious. Try setting break points and see if any of them hit?
If they don't hit, the entry from JS is probably here:
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/css/cssom/inline_style_property_map.cc;l=32