PDFiumPage::GetTextRunInfo() already has good heuristics to calculate
the set of characters that form a text run. Use it in
PDFiumRange::GetScreenRects() to create a rectangle for every text run.
Then add more heuristics to merge the rectangles when they have
sufficient overlap in the horizontal direction. As a result, the text
selection, which is based on GetScreenRects() calculations, will appear
more continuous and less jagged.
Update test expectations to match the new code's behavior.
Keep the original FPDFText_CountRects()-based implementation around,
behind an emergency kill switch, in case the new code has serious
problems.
Bug: 40448046
Change-Id: I17b9dd744671c6721faaf9060622cff788371f1f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6237116
Reviewed-by: Andy Phan <andyphan@chromium.org>
Reviewed-by: Alan Screen <awscreen@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1417508}
Currently, PDFiumPage tracks Searchified text at the granuarity of
words, but the tracking gets confused after PDFiumPage::Unload() gets
called, because the handles to the tracked text objects are no longer
valid. Given the assumption that a PDFiumPage either only contains text
from the PDF itself or only contains text from Searchify, simplify
Searchified text tracking from word granuarity to page granuarity. This
avoids the tracking confusion without requiring writing tags into the
PDF.
Bug: 376304020
Change-Id: I890b2be37ef7974b10920bc6b47126711d533d57
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6223665
Reviewed-by: Ramin Halavati <rhalavati@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1417182}
Refactor PDFiumEngine by using `PDFiumEngine::GetCurrentOrientation()`
instead of using `layout_.options().default_page_orientation()`, which
is the same thing.
This removes some redundancy and slightly improves readability.
Change-Id: I7cdf006b02aaf2a4377392e2e35acd15b5b2447e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6235350
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Andy Phan <andyphan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1416852}
Currently, PDFiumRange::SetCharCount() will unconditionally mark the
PDFiumRange instance's cached screen rects as dirty. This is unnecessary
if the character count has not changed. Check for this condition and
turn SetCharCount() into a no-op.
Change-Id: I699d916b155d8e237774d7b408474e304bd07daa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6230982
Reviewed-by: Andy Phan <andyphan@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1416281}
Fix a crash where PdfInkModule::RecordStrokePosition() complains the
inputs are made with different tool types. Check the tool type and
ignore the event if the types are different.
Bug: 391387325
Change-Id: If5fec5a9ca5e4d2aa3a6d5c95f7194ba8a53e917
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6195922
Reviewed-by: Alan Screen <awscreen@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1415108}
Add a new test PDF where there is an empty page in the middle. Use it in
a new PDFiumEngineTest.SelectTextAcrossEmptyPage test case to exercise
more PDFiumEngine code.
Change-Id: I2b91595005cdac9d2272d481cdcdd7b6cc637f7a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6221114
Reviewed-by: Ramin Halavati <rhalavati@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1415045}
`PDFiumEngine::SelectAll` selected all pages regardless of having text
or not, while `PDFiumEngine::Selection` only selects pages in the given
range that include text.
To make the behavior consistent, update the former to only select pages
with text.
Bug: 387387738, 392926460
Change-Id: Iee4833d7b44acc8e5ef7175560f77de0b4a1ed25
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6218255
Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1414767}
This IPC message to the browser ends up as a no-op, as the browser no
longer displays an infobar in response. Remove the IPC, considering it
has been a no-op for nearly a decade.
Change-Id: I673cd8a2558b4a84afd5d573c020ff701a3c237e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6220800
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1414512}
If a page is in progressive paint, committing searchify results can
crash PDFium. Hence delay adding the changes to the page until paint
completes for that page.
Bug: 392885625
Change-Id: I1d052985a7bdfbe98bd8f9d3051d1a28b031f3d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6218539
Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1413919}
This refactors code to draw the PDF selection box highlights and
scrolling to bounding rects into functions that can be shared. This is a
precursor CL to rendering and scrolling to text fragment highlights.
Change-Id: I9e5c0d05a270d7ec930efba505cc9a88b4ab3a75
Bug: 383575917
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6219663
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Juan Mojica <juanmojica@google.com>
Cr-Commit-Position: refs/heads/main@{#1413793}
Make the behavior in this scenario more consistent with other apps.
Change PdfInkModule's to remember if it ever received a pen event, and
ignore touch events after that. Update tests with the new expectations,
add ApplyStrokeWithTouchAtPointsNotHandled() as needed, and remove the
now fulfilled TODOs.
Bug: 392650039
Change-Id: I38c45d8fc369748c49619e3448f713651e5e3797
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6216548
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Alan Screen <awscreen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1413686}
Add a test case for a scenario where the sequence of events is a mix of
touch and pen events. Set the expectations to the current behavior, and
add TODOs for getting to the desired behavior.
Bug: 392650039
Change-Id: I1f51aa1c2754b933ad394e980fca0e8f7e8fe4ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6216546
Reviewed-by: Alan Screen <awscreen@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1413679}
GetToolTypeFromTouchEvent() has a TODO that assumes it has to deal with
multi-touch, but it turns out all the callers already rejected
multi-touch events. As such, remove the TODO and simplify
GetToolTypeFromTouchEvent() to just check the first and only touch.
Bug: 377733396
Change-Id: I7d9b19c6ac3f341106b39d9d64287f97eaa0e83f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6216547
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Andy Phan <andyphan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1413675}
- Use the existing EnableAnnotationMode() helper instead of
re-implementing it.
- Add the check to make sure PdfInkModule is enabled to
EnableAnnotationMode(), instead of making the callers do it.
Change-Id: I551fd1bd72698def36e8422c4a5841ff20270d69
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6217489
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Alan Screen <awscreen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1413641}
Since Result is an enum and not an enum class, it is very easy to mix up
this enum and ints. To prepare for making Result an enum class, start
using it in some callbacks. This forces the compiler to be stricter
about type checking. Then use Result in more places to fix the compile
errors. For url_loader_unittest.cc in particular, split the existing
mock callback variable into two separate variables for open and read
operations.
Change-Id: I1c830fd88aba39542fa60508d9e1c26ed1744e5c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6216483
Reviewed-by: Andy Phan <andyphan@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1413268}
Instead of taking an int, the callback OpenRange() runs only needs a
boolean to indicate success. Update all related code to pass in a
boolean. Then a success value of true is easier to understand than a
result value of Result::kSuccess, which is often written as 0. This
cleanup means less work in a future CL to convert Result from enum to
enum class.
Change-Id: I00589ca711721f586899325a01fca196eb8d33bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6216499
Reviewed-by: Andy Phan <andyphan@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1413256}
Replace WebVector (which has been an alias of std::vector since
crrev.com/c/6204022) with std::vector.
function cg() {
git grep --name-only "$1" -- *.mm *.h *.cc
}
sed -i 's/include.*web_vector.h"/include <vector>/' `cg web_vector.h`
sed -i '/^using blink::WebVector;/d' `cg 'using blink::WebVector'`
sed -i '/^using ::blink::WebVector;/d' `cg 'using ::blink::WebVector'`
sed -i 's/blink::WebVector\b/std::vector/g' `cg blink::WebVector`
sed -i 's/\bWebVector\b/std::vector/g' `cg WebVector`
git checkout -- third_party/blink/public/platform/web_vector.h
git cl format
Only manual changes are to remove unused "#include <vector>" added by
the above script, based on presubmit warnings.
BYPASS_LARGE_CHANGE_WARNING=`git cl split` will create too many CLs needing too many reviewers while this CL doesn't need much manual review.
Bug: 40865165
Change-Id: Ib70af44863ceaa73f470c77abe74d8994e2822a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6209609
Owners-Override: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1412669}
Since https://crrev.com/1380766 landed, PdfInkModule::Draw() only has
work to do if there is an in-progress stroke. Rendering for all
previous strokes are handled as objects within the PDF document.
Introduce a new query so that callers can determine if there is
anything for PdfInkModule to draw. This allows the caller to reduce
the overhead for drawing in the common case where there is currently
no active stroke being drawn.
Also update the PdfInkModule::Draw() API comment to reflect what it
currently draws, and also its requirements given that it can now expect
only to be called for an in-progress stroke.
Bug: 391666319
Change-Id: Ie6a89cfde9a3893e05ac10852f91550c64b9f093
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6191591
Reviewed-by: Lei Zhang <thestig@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Commit-Queue: Alan Screen <awscreen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1410558}
If a PDF is searchified, the extracted text is written to the file when
the PDF is saved.
This is done behind `chrome_pdf::features::kPdfSearchifySave` flag which
is disabled by default.
In a next CL (http://crrev.com/c/6110470), the user will be given the
option to save the original PDF or the PDF with extracted text.
Bug: 382610226
Change-Id: I73e1028ce60211e8bed165ffaff7f6630c9103c4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6049156
Reviewed-by: Andy Phan <andyphan@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Alan Screen <awscreen@chromium.org>
Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1407724}
In cases where the viewport is too large, an image buffer allocation
will fail and cause a crash inside SkBitmap::allocPixels(). Switch to
SkBitmap::tryAllocPixels() to avoid this crash. Presumably when the
viewport gets resized back to a normal size, OnViewportChanged() will
run again, successfully allocation the buffer, and the PDF Viewer will
recover.
Bug: 386572857
Change-Id: I1db92e2aed0b515316636e408c2942daf9ce4f65
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6180508
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1407620}
Initially, all touch and pen events were treated as touch events and
used a touch tool type for strokes. Instead, identify when the touch
event is actually a pen event and use the pen tool type for pen events.
Note that WebPluginContainerImpl::HandleEvent() treats pen events as
blink::WebTouchEvents for compatibility, which is why this distinction
needs to be made when handling blink::WebTouchEvents in
pdf_ink_module.cc.
Bug: 377733396, 380433757
Change-Id: I8a434a4998b44f934c18fe26c003469d1763095a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6168829
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Commit-Queue: Andy Phan <andyphan@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1406307}
A user can simultaneously apply different modes of input (such as mouse
+ keyboard, or mouse + touchscreen) to cause a change in tool state in
the middle of a drawing stroke.
Modify PdfInkModule so that the current drawing brush can not be
changed while there is an in-progress drawing stroke. Any changes to
drawing tool state is captured and applied after an in-progress stroke
is finished, making the changes only applicable to subsequent strokes.
This includes delaying an update to the cursor image, so that it
consistently matches the PdfInkBrush state.
Bug: 381908888
Change-Id: Icbeb342b9c3d7e614be6ee044c7355a3041c4a44
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6103003
Reviewed-by: Lei Zhang <thestig@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Andy Phan <andyphan@chromium.org>
Commit-Queue: Alan Screen <awscreen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1405717}
The "structure tree" inside a PDF file provides information that can be
used by assistive software to expose the organization and semantics of
the file's elements, e.g. indicate where the headings are, or present
some textual information as a table.
PDFs with a "structure tree" are called "tagged".
This patch starts the journey of supporting tagged PDFs by exposing the
tag's type, i.e. the accessibility role, of every text span to assistive
software. This allows, e.g. headings, and list bullets to be supported.
Design doc at:
https://docs.google.com/document/d/1ScD93clMA7AtViWINnaQgRTAK3vbpMshQZQXmWkfCyQ/edit?usp=sharing
AX-Relnotes: n/a.
Change-Id: Id56a8d6e21196c383b0d38ceac24923bf5fadd86
Bug: 40707542
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5980169
Reviewed-by: Ramin Halavati <rhalavati@chromium.org>
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Reviewed-by: Kyungjun Lee <kyungjunlee@google.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1405381}
This CL adds a method to get the current visible PDF page. In a follow
up CL, I'll use the new page index to grab text from the
X pages after the one the user is viewing and include it in the partial
upload request.
Bug: 387306854
Change-Id: Ibc145a5c7cb5f359614814393d671f3fea460299
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6134412
Reviewed-by: Matthew Denton <mpdenton@chromium.org>
Reviewed-by: Ali Stanfield <stanfield@google.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Duncan Mercer <mercerd@google.com>
Cr-Commit-Position: refs/heads/main@{#1405010}
A user can simultaneously apply different modes of input (such as mouse
+ keyboard, or mouse + touchscreen) to cause a change in tool state in
the middle of a drawing stroke.
Add a test for changing the drawing brush type in the middle of an
in-progress stroke, such as from the pen to the highlighter. This
causes the entire stroke to change with the updated state, which is not
the desired behavior.
Bug: 381908888
Change-Id: I908c252d7b08868a277e96bb2673fd7661fea088
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6115082
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Andy Phan <andyphan@chromium.org>
Commit-Queue: Alan Screen <awscreen@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1399470}
d21aa587ba..e5673a4ff2
$ git log d21aa587b..e5673a4ff --date=short --no-merges --format='%ad %ae %s'
2024-11-12 sfreilich Rename ModeledShape to PartitionedMesh
Created with:
roll-dep src/third_party/ink/src
Then update Chromium build rules, includes, and type references to match
the Ink rename. Variables still keep their "shape" names, as they do in
Ink.
Change-Id: I2f390013590d505c1c9dcc5f5c5cdfa35a4a327b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6112871
Reviewed-by: Alan Screen <awscreen@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1399420}
A user can simultaneously apply different modes of input (such as mouse
+ keyboard, or mouse + touchscreen) to cause a change in tool state in
the middle of a drawing stroke.
When a user changes the size of the eraser in the middle of an eraser
stroke, the internal eraser state is getting overwritten in such a way
that the PDF viewer will crash if a user performs a subsequent stroke.
Add protection in PdfInkModule to avoid overwriting the state so that a
crash is avoided.
This scenario also illustrates that changes to the eraser size gets
immediately applied to an in-progress stroke. This is not the desired
behavior, as such changes should only be applied to subsequent strokes.
Add a TODO to note that this behavior still needs to be addressed.
Bug: 381908888
Change-Id: I782420ec24893ba85227c3f70d13e8deddcee118
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6103002
Commit-Queue: Alan Screen <awscreen@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Andy Phan <andyphan@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1398958}
A user can simultaneously apply different modes of input (such as mouse
+ keyboard, or mouse + touchscreen) to cause a change in tool state in
the middle of a drawing stroke.
When a user changes the color or size of a pen/highlighter in the
middle of a drawing stroke, the internal drawing state is getting
overwritten in such a way that the PDF viewer will crash if a user
performs a subsequent stroke. Add protection in PdfInkModule to avoid
overwriting the state so that a crash is avoided.
This scenario also illustrates that changes to the pen color or size
get immediately applied to an in-progress stroke. This is not the
desired behavior, as such changes should only be applied to subsequent
strokes. Add TODOs to note that this behavior still needs to be
addressed.
Bug: 381908888
Change-Id: I254b65bbb05090415a73adb28bcfc9e4be8e24f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6102042
Reviewed-by: Lei Zhang <thestig@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Andy Phan <andyphan@chromium.org>
Commit-Queue: Alan Screen <awscreen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1398955}
A user can simultaneously apply different modes of input (such as mouse
+ keyboard, or mouse + touchscreen) to cause a change in tool state in
the middle of a draw or erase stroke. Such actions should not cause
the PDF viewer to crash.
Avoid this crash by automatically finishing the in-progress stroke at
the time of the tool type change. The event which would normally
signal to finish the stroke is now considered unhandled since the
stroke already finished.
Bug: 381908888
Change-Id: Ia5690e271886d82864fd155bd09c00da7b9d7a37
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6103525
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Alan Screen <awscreen@chromium.org>
Reviewed-by: Andy Phan <andyphan@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1398878}
