Convert some {Byte,Wide}String::Last() calls to Substr(). [pdfium : main]
1 view
Skip to first unread message
Tom Sepez (Gerrit)
Apr 7, 2026, 7:15:42 PM (4 days ago) Apr 7
to Pdfium LUCI CQ, pdfium-...@googlegroups.com
Tom Sepez voted and added 1 comment![]( "Open in Gerrit")
Votes added by Tom Sepez
| Commit-Queue | +1 |
1 comment
File core/fpdfapi/page/cpdf_streamcontentparser.cpp
Line 1269, Patchset 1:
defname += name.Substr(7);Tom Sepez . resolved
This is a perfectly accurate mechanical translation of the original code, but it seems to highlight a pre-existing bug!
`name` starts with the 6-character prefix "Device" (as seen in the `if` condition above).
`Substr(7)` skips the first 7 characters. For "DeviceGray" (length 10), it skips the 'G' at index 6 and returns "ray", resulting in `defname` becoming "Defaultray".
For "DeviceRGB", it results in "DefaultGB", and for "DeviceCMYK" it results in "DefaultMYK".
The intent here is clearly to look up "DefaultGray", "DefaultRGB", and "DefaultCMYK". The author of the original code likely used `7` by mistake, perhaps confusing the length of "Default" (7) with the length of "Device" (6).
Since you're modifying this line, consider fixing this by changing it to `name.Substr(6)`.
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Lei Zhang (Gerrit)
Apr 7, 2026, 7:24:24 PM (4 days ago) Apr 7
to Tom Sepez, Lei Zhang, Pdfium LUCI CQ, pdfium-...@googlegroups.com
Attention needed from Tom Sepez
Lei Zhang added 1 comment![]( "Open in Gerrit")
Commit Message
Line 12, Patchset 3 (Latest):
-- Fix one suggested length botch,Lei Zhang . unresolved
Can that be a separate CL?
Related details
Attention is currently required from:
- Tom Sepez
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. |
Tom Sepez (Gerrit)
Apr 7, 2026, 7:27:47 PM (4 days ago) Apr 7
to Lei Zhang, Pdfium LUCI CQ, pdfium-...@googlegroups.com
Attention needed from Lei Zhang
Tom Sepez added 1 comment![]( "Open in Gerrit")
Commit Message
Can that be a separate CL?
Attention is currently required from:
- Lei Zhang
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Lei Zhang (Gerrit)
Apr 7, 2026, 7:40:10 PM (4 days ago) Apr 7
to Tom Sepez, Lei Zhang, Pdfium LUCI CQ, pdfium-...@googlegroups.com
Attention needed from Tom Sepez
Lei Zhang voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Tom Sepez
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tom Sepez (Gerrit)
Apr 7, 2026, 7:40:34 PM (4 days ago) Apr 7
to Lei Zhang, Pdfium LUCI CQ, pdfium-...@googlegroups.com
Tom Sepez voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Pdfium LUCI CQ (Gerrit)
Apr 7, 2026, 9:16:28 PM (3 days ago) Apr 7
to Tom Sepez, Lei Zhang, pdfium-...@googlegroups.com
Pdfium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
Convert some {Byte,Wide}String::Last() calls to Substr().
Gemini generated patch once human realized that many of these
less-readable expressions abound.Change-Id: Id0baf48847395be56b15f0e4187bdeecd91b3fd4
Reviewed-by: Lei Zhang <the...@chromium.org>
Commit-Queue: Tom Sepez <tse...@chromium.org>
Files:
- M core/fpdfapi/edit/cpdf_pagecontentgenerator_unittest.cpp
- M core/fpdfapi/font/cpdf_cmapparser.cpp
- M core/fpdfapi/font/cpdf_fontglobals.cpp
- M core/fpdfapi/page/cpdf_streamcontentparser.cpp
- M core/fxcrt/css/cfx_cssselector.cpp
- M core/fxcrt/fx_string.h
- M core/fxcrt/xml/cfx_xmlelement.cpp
- M core/fxge/cfx_fontmapper.cpp
- M fxjs/cjs_publicmethods.cpp
- M fxjs/cjs_util.cpp
- M fxjs/xfa/cfxjse_engine.cpp
- M fxjs/xfa/cfxjse_formcalc_context.cpp
- M fxjs/xfa/cfxjse_nodehelper.cpp
- M fxjs/xfa/cfxjse_resolveprocessor.cpp
- M fxjs/xfa/cjx_instancemanager.cpp
- M fxjs/xfa/cjx_subform.cpp
- M xfa/fgas/crt/cfgas_stringformatter.cpp
- M xfa/fxfa/cxfa_eventparam.cpp
- M xfa/fxfa/formcalc/cxfa_fmexpression.cpp
- M xfa/fxfa/layout/cxfa_viewlayoutprocessor.cpp
- M xfa/fxfa/parser/cxfa_document_builder.cpp
- M xfa/fxfa/parser/cxfa_localevalue.cpp
- M xfa/fxfa/parser/cxfa_node.cpp
Change size: M
Delta: 23 files changed, 41 insertions(+), 50 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Lei Zhang
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages