Take the position of the selected element into account when scrolling matches (issue 13596)
#13600
Conversation
…ng matches (issue 13596) Note that as far as I can tell, this is *not* a regression but rather a bug which has existed since basically "forever". **In order to reproduce this easily:** - Open the viewer. - Set the zoom level to `400%`, - Search for "expression". The problem here is that when scrolling matches into view, we're scrolling to the start of the *containing* `textLayer` element rather than the start of the highlighted match itself.[1] When the entire width (or at least most) of the page is visible in the viewer, that doesn't really matter though which is likely why this bug has gone unnoticed for so long.[2] Given that the highlighted match can be placed anywhere, e.g. even at the very end, within its `textLayer` element it's quite easy to see why the current implementation becomes a problem at higher zoom levels. All of this is then *further* exacerbated by `PDFFindController.scrollMatchIntoView` using a negative left offset, to ensure that the current match has some (visible) context available once scrolled into view. In order to address this long-standing bug, we'll determine the (left) offset of the `selected` match and use that to modify the final position scrolled to in `PDFFindController.scrollMatchIntoView` such that the match is visible regardless of zoom level. --- [1] Unfortunately we cannot directly scroll to the `selected` match, since it's not absolutely positioned and changing that would cause other bugs/regressions (note recent patches in that area). [2] I did actually stumble upon this problem a little while ago, while working on PR 13482, but forgot to look into this again until I saw the new issue.
|
/botio integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/59c305804bbc834/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://3.101.106.178:8877/2751aa63e92bea9/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/59c305804bbc834/output.txt Total script time: 4.35 mins
|
From: Bot.io (Windows)FailedFull output at http://3.101.106.178:8877/2751aa63e92bea9/output.txt Total script time: 5.36 mins
|
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/4e5fed95b108c3f/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/4e5fed95b108c3f/output.txt Total script time: 4.79 mins Published |
|
I can't reproduce this issue using Firefox on Linux, so I'm wondering if this is somehow Windows-specific? The steps to reproduce above and in the issue itself don't lead to the word "expression" ever being out of view for me. Once the word is found, the document is scrolled to the right position, even when it's out of view... This is right after I opened the viewer (on automatic zoom) and changed it to 400%:
and then this is what I see immediately after I search for "expression":
|
I don't think it's Windows specific, but looking at your screen-shot (which is very helpful) I've got an idea that might explain this :-) Most likely it's also connected to the width of the browser window, since that'll (in hindsight) obviously affect things. Could you perhaps try to increase the zoom level even more, to e.g. |
|
I tried again with a much smaller window and that worked; thanks! |
Note that as far as I can tell, this is not a regression but rather a bug which has existed since basically "forever".
In order to reproduce this easily:
400%,The problem here is that when scrolling matches into view, we're scrolling to the start of the containing
textLayerelement rather than the start of the highlighted match itself.[1] When the entire width (or at least most) of the page is visible in the viewer, that doesn't really matter though which is likely why this bug has gone unnoticed for so long.[2]Given that the highlighted match can be placed anywhere, e.g. even at the very end, within its
textLayerelement it's quite easy to see why the current implementation becomes a problem at higher zoom levels. All of this is then further exacerbated byPDFFindController.scrollMatchIntoViewusing a negative left offset, to ensure that the current match has some (visible) context available once scrolled into view.In order to address this long-standing bug, we'll determine the (left) offset of the
selectedmatch and use that to modify the final position scrolled to inPDFFindController.scrollMatchIntoViewsuch that the match is visible regardless of zoom level.[1] Unfortunately we cannot directly scroll to the
selectedmatch, since it's not absolutely positioned and changing that would cause other bugs/regressions (note recent patches in that area).[2] I did actually stumble upon this problem a little while ago, while working on PR 13482, but forgot to look into this again until I saw the new issue.