[hanging-punctuation] add parsing logic [chromium/src : main]

[Lingqi Chi (Gerrit)]

Attention needed from Koji Ishii

Lingqi Chi voted and added 2 comments

Votes added by Lingqi Chi

Commit-Queue

+1

2 comments

Patchset-level comments

File-level comment, Patchset 5 (Latest):

Lingqi Chi . resolved

PTAL 😊

File third_party/blink/renderer/core/css/css_properties.json5

Line 3600, Patchset 5 (Latest): converter: "ConvertHangingPunctuation",

Lingqi Chi . unresolved

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?

https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/css/cssom/style_property_map_test.cc;drc=d6839a01f38435301ede36c7b979898e14a8c6ff;l=102

Open in Gerrit

Related details

Attention is currently required from:

• Koji Ishii

Submit Requirements:

• 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.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I66fc6e6cc83634924a81bf3ec96b7b150090eb1e

Gerrit-Change-Number: 7221572

Gerrit-PatchSet: 5

Gerrit-Owner: Lingqi Chi <lin...@chromium.org>

Gerrit-Reviewer: Koji Ishii <ko...@chromium.org>

Gerrit-Reviewer: Lingqi Chi <lin...@chromium.org>

Gerrit-CC: AyeAye <android-bu...@system.gserviceaccount.com>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: Menard, Alexis <alexis...@intel.com>

Gerrit-CC: chromium...@chromium.org

Gerrit-Attention: Koji Ishii <ko...@chromium.org>

Gerrit-Comment-Date: Tue, 09 Dec 2025 07:46:47 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

[Koji Ishii (Gerrit)]

Attention needed from Lingqi Chi

Koji Ishii added 1 comment

File third_party/blink/renderer/core/css/css_properties.json5

Line 3600, Patchset 5: converter: "ConvertHangingPunctuation",

Lingqi Chi . unresolved

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?

https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/css/cssom/style_property_map_test.cc;drc=d6839a01f38435301ede36c7b979898e14a8c6ff;l=102

Koji Ishii

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?

Open in Gerrit

Related details

Attention is currently required from:

• Lingqi Chi

Submit Requirements:

• 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.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: chromium/src

Gerrit-Branch: main

Gerrit-Change-Id: I66fc6e6cc83634924a81bf3ec96b7b150090eb1e

Gerrit-Change-Number: 7221572

Gerrit-PatchSet: 6

Gerrit-Owner: Lingqi Chi <lin...@chromium.org>

Gerrit-Reviewer: Koji Ishii <ko...@chromium.org>

Gerrit-Reviewer: Lingqi Chi <lin...@chromium.org>

Gerrit-CC: AyeAye <android-bu...@system.gserviceaccount.com>

Gerrit-CC: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>

Gerrit-CC: Menard, Alexis <alexis...@intel.com>

Gerrit-CC: chromium...@chromium.org

Gerrit-Attention: Lingqi Chi <lin...@chromium.org>

Gerrit-Comment-Date: Sat, 20 Dec 2025 15:22:05 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Lingqi Chi <lin...@chromium.org>

[Koji Ishii (Gerrit)]

Attention needed from Lingqi Chi

Koji Ishii added 1 comment

File third_party/blink/renderer/core/css/css_properties.json5

Line 3600, Patchset 5: converter: "ConvertHangingPunctuation",

Lingqi Chi . unresolved

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?

https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/css/cssom/style_property_map_test.cc;drc=d6839a01f38435301ede36c7b979898e14a8c6ff;l=102

Koji Ishii

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?

Koji Ishii

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.

Gerrit-Comment-Date: Mon, 22 Dec 2025 10:13:47 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Lingqi Chi <lin...@chromium.org>

Comment-In-Reply-To: Koji Ishii <ko...@chromium.org>