mozilla / pdf.js

PDF Reader in JavaScript
https://mozilla.github.io/pdf.js/
Apache License 2.0
47.06k stars 9.81k forks source link

[BUG] Can not create text annotation in mobile browser #18095

Open aosyatnik opened 1 month ago

aosyatnik commented 1 month ago

Use sample pdf https://mozilla.github.io/pdf.js/web/viewer.html or https://mozilla.github.io/pdf.js/legacy/web/viewer.html .

Steps to reproduce the problem:

  1. In mobile device, I was using Samsung S22, open sample pdf with chrome browser. Same issue is in Mozilla browser.
  2. Try to add text annotation.
  3. Text annotation is hidden before you even write any text.

What went wrong? (add screenshot) Video_2024_05_15-1

shivaprsd commented 1 week ago

Let me add that the other annotation tools (especially the Stamp Editor) are also a bit buggy on mobile/touch screen. E.g. try adding a stamp annotation on Firefox Android and try to resize the image.

I know that the mobile platform is not the main target of PDF.js; just thought I would report when I saw this issue open.

avdoseferovic commented 1 week ago

my two cents: The reported issue is only present if the zoom is set to 'auto', 'page-width' or 'page-fit' and the reason for that is that the open soft keyboard causes an resize event, which further triggers https://github.com/mozilla/pdf.js/blob/06800cd966111f7262d3334dfb68c3765bc92740/web/app.js#L2361 and that triggers another resize event which closes the keyboard in the end.

I wonder, do we need to set pdfViewer.currentScaleValue = currentScaleValue; on resize event for mobile devices? The only reasonable way the size gets changed is when the devices screen orientation change (or am I wrong here totally)

@Snuffleupagus I could draft a PR that handles the resize event differently for mobile devices if you are up for that. basically I'd remove the setting of pdfViewer.currentScaleValue = currentScaleValue; on mobile devices in the resize handler, but implement another listener for https://developer.mozilla.org/en-US/docs/Web/API/ScreenOrientation and assign it there.

Snuffleupagus commented 1 week ago

I could draft a PR that handles the resize event differently for mobile devices if you are up for that. basically I'd remove the setting of pdfViewer.currentScaleValue = currentScaleValue; on mobile devices in the resize handler, but implement another listener for https://developer.mozilla.org/en-US/docs/Web/API/ScreenOrientation and assign it there.

Sorry, but to me that sounds more like a work-around rather than a real solution and we really want to avoid introducing "inconsistent" behaviour in the code.