mozilla / pdf.js

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

Chrome selects entire document when highlighting text. #9843

Open burtonator opened 6 years ago

burtonator commented 6 years ago

This problem has been existing for a while I think and hasn't yet been fixed on Chrome.

It's documented well here:

https://github.com/mozilla/pdf.js/issues/4843

And there seem to be a lot of patches that address this issue. However, on Chrome the 'select entire document' problem remains.

Basically, if you're selecting a paragraph, the text in the current paragraph will initially work fine. But then what happens is that you'll get between lines, and then the entire document will be highlighted, then move to the next line and it jumps back and only selects where your mouse pointer is.

I can create a video if you want but if you just try to select text slowly on chrome it will exhibit this behavior.

We're building an annotation app on top of PDF.js and this prevents us from easily highlighting text.

Attach (recommended) or Link to PDF file here:

https://mozilla.github.io/pdf.js/web/viewer.html

Configuration:

Steps to reproduce the problem:

  1. Launch chrome and load pdf
  2. Try to select text, if you drag slowly between lines, the entire document is selected.

What is the expected behavior? (add screenshot)

Same behavior on firefox.

What went wrong? (add screenshot)

Link to a viewer (if hosted on a site other than mozilla.github.io/pdf.js or as Firefox/Chrome extension):

https://mozilla.github.io/pdf.js/web/viewer.html

burtonator commented 6 years ago

Any way I could help here? This is a super important bug for us...

playtowork commented 5 years ago

FWIW, I have the same problem and would appreciate if anyone knows a solution for this.

ftc95 commented 5 years ago

Fixed: just add height:200px to .textLayer > div {height:200px;} in viewer.css

burtonator commented 5 years ago

Just tested it... it SEEMS to make the problem better but doesn't completely rectify it.

bikb commented 5 years ago

Little workaround:

if (sel.anchorNode) {   
  if (sel.anchorNode.className) {
    if (sel.anchorNode.className.match("textLayer") == "textLayer") {
       if (window.getSelection) {
          if (window.getSelection().empty) {  // Chrome
              window.getSelection().empty();
           } else if (window.getSelection().removeAllRanges) {  // Firefox
             window.getSelection().removeAllRanges();
           }
           } else if (document.selection) {  // IE?
             document.selection.empty();
           }
           $("#viewer").css({"-moz-user-select": "none"});
           console.log("selection off for 4 sec");
           wndow.setTimeout(function() {$("#viewer").css({"-moz-user-select": "text"});}, 4000);
           };
        }
    }

As event listener onselectionchange...

burtonator commented 5 years ago

This is really a bug that needs to be fixed... its impacting multiple people.

Any way I can help mozilla?

burtonator commented 5 years ago

@bikb on what element shouId I try that selectionchange listener?

burtonator commented 5 years ago

Just FYI this is impacting a LOT of our users:

https://getpolarized.io/

... ALL... ? Chrome is the major browser and 85% of our users are on Chrome and using this. Love that it works in FF but would like to see it working on Chrome too.

bikb commented 5 years ago

simply put it in: document.addEventListener('selectionchange', function() { }

bikb commented 5 years ago

change the -moz-user-select to user-select because my fix was for firefox and you need it for chrome

burtonator commented 5 years ago

@bikb thats ok but what is 'sel' ?I assume this is just window.getSelection() ?

burtonator commented 5 years ago

Also, I think it just clears the selection ... right?

burtonator commented 5 years ago

I think something like this would work.

Basically we prevent the default event handler when the selection is active and it's jumped into the parent.

Needs to be adjusted..

This stuff is prone to error so am I missing anything here?

https://gist.github.com/burtonator/1a097fe5c0b8212a84e83cba8e319773

One other feature I think is that the range should be extended based on the mouse position.

I think we could use elementsAtPoint based on the mouse position and changing the range but I still need to think about it.

lawriebaberscovell-solirius commented 4 years ago

The selection behaviour is also the same on I.E. 11.

I E  11 - Highlight

lawriebaberscovell-solirius commented 4 years ago

And on Safari...

Screenshot 2019-10-02 at 09 39 12

lawriebaberscovell-solirius commented 4 years ago

The problem is also on Firefox - 69.0.1 (64-bit). Windows 10 Pro.

Start selection right of the 'Abstract' title. Drag down. This is the result. When expecting start of the paragraph to be selected.

Firefox (1) - Highlight

lawriebaberscovell-solirius commented 4 years ago

Similar result if starting the selection left of the 'Abstract' title. Drag down.

lawriebaberscovell-solirius commented 4 years ago

Also, a problem when the mouse is dragged from the document to off the document space (but still within the viewer). The selection alters to select everything on the page prior the original starting point. Tested on Firefox - 69.0.1 (64-bit). Windows 10 Pro.

Firefox (2) - Highlight

Moved the mouse further right to trigger the problem.

Firefox (3) - Highlight

burtonator commented 4 years ago

It's funny that Mozilla complains that certain bugs are only fixed in Chrome but in one of their flagship OSS projects this bug has only been FIXED in Firefox for > 4 years now?

Can we figure out WHY it works in Firefox? If so we can implement a patch for the other browsers.

burtonator commented 4 years ago

@lawriebaberscovell-solirius and I'm unclear, are you saying your PR fixes this bug? Is so you're my hero!

timvandermeij commented 4 years ago

You can try the enhanced text selection mode, which is an experimental viewer preference. You can set the textLayerMode flag to 2 to enable it. Alternatively, go to https://mozilla.github.io/pdf.js/web/viewer.html, open the web console, run PDFViewerApplication.preferences.set('textLayerMode', 2); and refresh the page. I didn't notice a difference with regards to https://github.com/mozilla/pdf.js/issues/9843#issuecomment-537554926 and https://github.com/mozilla/pdf.js/issues/9843#issuecomment-537567313 in Firefox, but perhaps it does help in Chrome.

Note that this text selection mode is what is discussed in #4843 and that it's not enabled by default because it has some rough edges (most likely performance-wise since it does div expansion), which is tracked in #7584. If we know that this problem is reduced by it, we can perhaps take another shot at improving that so it can be enabled by default. If it doesn't fix it, we also have a bit more information to work with.

It's funny that Mozilla complains that certain bugs are only fixed in Chrome but in one of their flagship OSS projects this bug has only been FIXED in Firefox for > 4 years now?

Most of the behavior described here (although I think not everything has the same cause) happens in Firefox for me too, so I don't really know where this claim comes from, nor does it help to resolve the issue any sooner, so please keep comments limited to constructive ones.

lawriebaberscovell-solirius commented 4 years ago

@lawriebaberscovell-solirius and I'm unclear, are you saying your PR fixes this bug? Is so you're my hero!

No - it doesn't. But the problem appears in Firefox as well.

lawriebaberscovell-solirius commented 4 years ago

You can try the enhanced text selection mode, which is an experimental viewer preference. You can set the textLayerMode flag to 2 to enable it. Alternatively, go to https://mozilla.github.io/pdf.js/web/viewer.html, open the web console, run PDFViewerApplication.preferences.set('textLayerMode', 2); and refresh the page. I didn't notice a difference with regards to #9843 (comment) and #9843 (comment) in Firefox, but perhaps it does help in Chrome.

Note that this text selection mode is what is discussed in #4843 and that it's not enabled by default because it has some rough edges (most likely performance-wise since it does div expansion), which is tracked in #7584. If we know that this problem is reduced by it, we can perhaps take another shot at improving that so it can be enabled by default. If it doesn't fix it, we also have a bit more information to work with.

It's funny that Mozilla complains that certain bugs are only fixed in Chrome but in one of their flagship OSS projects this bug has only been FIXED in Firefox for > 4 years now?

Most of the behavior described here (although I think not everything has the same cause) happens in Firefox for me too, so I don't really know where this claim comes from, nor does it help to resolve the issue any sooner, so please keep comments limited to constructive ones.

Thanks Tim - We'll look into this.

omerGithub8 commented 2 years ago

Did somebody find a solution in chrome to this problem? This selection really makes the extension unusable, can't read properly. (using the latest pdf.js GitHub version)

chitgoks commented 2 years ago

Did somebody find a solution in chrome to this problem? This selection really makes the extension unusable, can't read properly. (using the latest pdf.js GitHub version)

did you try the textLayerMode - 2? i think its ok. but it has problem with regards to getting the text selection bounds since youll be getting bounds that are nor within the text's area.

if youre only concerned about selection, give that a try. but if you also need to get the bounds getClientRects, theyre whacked so its unusable.

ghost commented 2 years ago

@chitgoks @timvandermeij Is it possible to use the textLayerMode - 2 with the pre-compiled PDF.JS ? I mean the one available here https://mozilla.github.io/pdf.js/build/pdf.js because we have an application that does not use NodeJS. If possible, please provide sample code

jeager commented 2 years ago

Any news, progress, or recommendations on this one?

Although textLayerMode - 2 works to make the selection better, when we get the selected bounds (getClientRects) it is a mess.

jeager commented 1 year ago

@Snuffleupagus I understand closing the related issues, but would also be nice to share some perspective, if any, on this issue as well. Specially now that enhanceTextSelection has been removed.

seanaye commented 1 year ago

I am working on a fork for re-adding the enhanceTextSelection which was removed in #15259. Text selection is broken in Chromium without this feature.

If someone has found a better solution than this let me know

Snuffleupagus commented 1 month ago

Was this perhaps fixed, or at least improved, by PR #17923?

nicolo-ribaudo commented 1 month ago

The issue described in https://github.com/mozilla/pdf.js/issues/9843#issuecomment-537567313 still happens (in all browsers), while the rest of the examples that had a clear description on what to select and how to move the mouse seem to be fixed.