Refine whitespace stripping in CPDF_TextPage [pdfium : main]

[Aryan Krishnan (Gerrit)]

Aryan Krishnan voted and added 1 comment

Votes added by Aryan Krishnan

Commit-Queue

+1

1 comment

File fpdfsdk/fpdf_text_embeddertest.cpp

Line 2386, Patchset 5 (Latest): EXPECT_EQ(3051, FPDFText_CountChars(textpage.get()));

Aryan Krishnan . unresolved

I checked with a healthy amount of logging; we're successfully nuking the spaces, even if the CRLFs are still hanging around to keep the count at 3051.

My laptop’s fans are currently staging a protest against the chromium build process (although PDFium builds and tests pass). Can someone verify this looks good on the chromium-end. Thanks!

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

• 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: pdfium

Gerrit-Branch: main

Gerrit-Change-Id: I6e3ab9ff2530c24bc5ed65b59670fc9ed639622d

Gerrit-Change-Number: 143472

Gerrit-PatchSet: 5

Gerrit-Owner: Aryan Krishnan <aryankr...@gmail.com>

Gerrit-Reviewer: Aryan Krishnan <aryankr...@gmail.com>

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

Gerrit-Comment-Date: Fri, 20 Feb 2026 14:10:32 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

[Aryan Krishnan (Gerrit)]

Aryan Krishnan added 1 comment

File fpdfsdk/fpdf_text_embeddertest.cpp

Line 2386, Patchset 5 (Latest): EXPECT_EQ(3051, FPDFText_CountChars(textpage.get()));

Aryan Krishnan . unresolved

I checked with a healthy amount of logging; we're successfully nuking the spaces, even if the CRLFs are still hanging around to keep the count at 3051.

My laptop’s fans are currently staging a protest against the chromium build process (although PDFium builds and tests pass). Can someone verify this looks good on the chromium-end. Thanks!

Aryan Krishnan

Edit: We are removing the additional spaces at the top (causing the regression) the other spaces remain as is and rest should work as expected. Not 100% sure if the regression is fixed when building chromium. Please let me know if building the full chromium with this fixes this regression. Thanks!

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

• 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: pdfium

Gerrit-Branch: main

Gerrit-Change-Id: I6e3ab9ff2530c24bc5ed65b59670fc9ed639622d

Gerrit-Change-Number: 143472

Gerrit-PatchSet: 5

Gerrit-Owner: Aryan Krishnan <aryankr...@gmail.com>

Gerrit-Reviewer: Aryan Krishnan <aryankr...@gmail.com>

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

Gerrit-Comment-Date: Fri, 20 Feb 2026 14:14:41 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Aryan Krishnan <aryankr...@gmail.com>

[Aryan Krishnan (Gerrit)]

Attention needed from Andy Phan, Lei Zhang and Tom Sepez

Aryan Krishnan added 1 comment

File fpdfsdk/fpdf_text_embeddertest.cpp

Line 2386, Patchset 5 (Latest): EXPECT_EQ(3051, FPDFText_CountChars(textpage.get()));

Aryan Krishnan . resolved

I checked with a healthy amount of logging; we're successfully nuking the spaces, even if the CRLFs are still hanging around to keep the count at 3051.

My laptop’s fans are currently staging a protest against the chromium build process (although PDFium builds and tests pass). Can someone verify this looks good on the chromium-end. Thanks!

Aryan Krishnan

Edit: We are removing the additional spaces at the top (causing the regression) the other spaces remain as is and rest should work as expected. Not 100% sure if the regression is fixed when building chromium. Please let me know if building the full chromium with this fixes this regression. Thanks!

Aryan Krishnan

Resolving this. Reopen if it is still regressing on the full chromium build.

Open in Gerrit

Related details

Attention is currently required from:

• Andy Phan
• Lei Zhang
• Tom Sepez

Submit Requirements:

• Code-Owners
• Code-Review

• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: pdfium

Gerrit-Branch: main

Gerrit-Change-Id: I6e3ab9ff2530c24bc5ed65b59670fc9ed639622d

Gerrit-Change-Number: 143472

Gerrit-PatchSet: 5

Gerrit-Owner: Aryan Krishnan <aryankr...@gmail.com>

Gerrit-Reviewer: Andy Phan <andy...@chromium.org>

Gerrit-Reviewer: Aryan Krishnan <aryankr...@gmail.com>

Gerrit-Reviewer: Lei Zhang <the...@chromium.org>

Gerrit-Reviewer: Tom Sepez <tse...@chromium.org>

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

Gerrit-Attention: Lei Zhang <the...@chromium.org>

Gerrit-Attention: Tom Sepez <tse...@chromium.org>

Gerrit-Attention: Andy Phan <andy...@chromium.org>

Gerrit-Comment-Date: Fri, 20 Feb 2026 14:15:30 +0000

[Aryan Krishnan (Gerrit)]

Attention needed from Andy Phan, Lei Zhang and Tom Sepez

Aryan Krishnan added 1 comment

Patchset-level comments

File-level comment, Patchset 5 (Latest):

Aryan Krishnan . resolved

Would abandon and make this a reland so that it avoids the merge conflicting.

Gerrit-Comment-Date: Fri, 20 Feb 2026 18:26:27 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Aryan Krishnan (Gerrit)]

Aryan Krishnan abandoned this change.

View Change

Abandoned Make this a reland of the original CL.

Aryan Krishnan abandoned this change

Related details

Attention set is empty

Submit Requirements:

• Code-Review
• Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: abandon