[Snuffleupagus]

This property was added in PR #13494 specifically for the debugger, however it wasn't actually used anywhere else.
More recently PR #20334 started using that property in getTextContent, however it's not clear to me that doing so was necessarily correct. First of all, note that Glyph instances already contain an isSpace property. Secondly, and more importantly, using the "raw" charCode may be wrong in composite fonts with multi-byte strings.

Note how the isSpace property is determined in the PDF.js code:

pdf.js/src/core/fonts.js

Lines 3721 to 3732 in 8e62d24

	// Space is char with code 0x20 and length 1 in multiple-byte codes.
	const glyph = this._charToGlyph(
	charcode,
	length === 1 && chars.charCodeAt(i - 1) === 0x20
	);
	glyphs.push(glyph);
	}
	} else {
	for (let i = 0, ii = chars.length; i < ii; ++i) {
	const charcode = chars.charCodeAt(i);
	const glyph = this._charToGlyph(charcode, charcode === 0x20);
	glyphs.push(glyph);

which is based on the following specification text:

Word spacing shall be applied to every occurrence of the single-byte character code 32 in a string when using
a simple font or a composite font that defines code 32 as a single-byte code. It shall not apply to occurrences of the byte value 32 in multiple-byte codes.

[github-actions]

Reference tests

Results for 7b0a38d (run, references from mozilla/pdf.js.refs@b11d8aa217).

Platform	Status	Tests	Total runtime	Errors	Different	FBF	No reference	Report
	⚠️	1411	11m 59s	0	1	0	0	view differences
	⚠️	1411	15m 41s	0	1	0	0	view differences

The report is kept for 30 days and replaced by the next run.

[github-actions]

Viewer preview

🗑️ Viewer previews removed.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.58%. Comparing base (8e62d24) to head (7b0a38d).

Files with missing lines	Patch %	Lines
src/core/fonts.js	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #22350      +/-   ##
==========================================
- Coverage   90.59%   90.58%   -0.01%     
==========================================
  Files         278      278              
  Lines       68535    68536       +1     
==========================================
- Hits        62088    62085       -3     
- Misses       6447     6451       +4     

Flag	Coverage Δ
browsertest	66.19% <75.00%> (-0.05%)	⬇️
fonttest	8.99% <0.00%> (-0.01%)	⬇️
integrationtest	69.55% <75.00%> (+<0.01%)	⬆️
unittest	61.26% <75.00%> (-0.01%)	⬇️
unittestcli	59.00% <75.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[codspeed]

Merging this PR will not alter performance

✅ 88 untouched benchmarks
⏩ 174 skipped benchmarks¹

---

_{Comparing Snuffleupagus:Glyph-conditional-originalCharCode (7b0a38d) with master (8e62d24)}

Footnotes

1. 174 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[Snuffleupagus]

While it's difficult to tell if the test "failures" are a problem just from the reference images, comparing issue15910.pdf locally in the viewer with textLayer-debugging enabled shows that this patch is an improvement:

• master

  

• This patch

  

[calixteman]

LGTM. Thank you.