Text input field is not always scrolled to top

[rugk]

Bug description

Also discovered in #199

Steps to reproduce

Steps to reproduce the behavior:

1. Go to https://longextendedsubdomainnamewithoutdashesinordertotestwordwrapping.badssl.com/#sdfhhhhhhhhhhhhhhhhhhhhhhhhhhhhhhaklshdiowhaodew902123423423423432erasd
2. Open the QR code popup…

Actual behavior:

The scroll bar is at the bottom, it's scrolled down.

Expected behavior:

Scroll bar should be at the top, i.e. it should display the beginning of the URL.

More details:

• Do also test different ways to trigger the QR code popup with large data. Ctrl+A and use the context
• Also test the setting in the options "Automatically use text selected on website.".

The reason that you need to test this is, that all three ways work differently in passing/getting the data to the QR code popup panel. They e.g. trigger at different times and you can expect race conditions or so…

FYI: opening via hot key is (should be) the same as via click/no difference is expected here.

System

Operation system and version: Linux/Fedora 30/GNOME
Browser and version: Firefox 69
Add-on version: 1.6

Further information

Responsible function:

offline-qr-code/src/popup/modules/UserInterface.js

Line 175 in 6e817de

function scrollToTop(event) {

For a solution, I may be fine with refactoring this (it's kinda needed) and possibly split it into it's own file (module). (together with some other text selection things there…)

[rugk]

@Rad0n You took this issue on. Could you also explain how you think the PR fixes the issue?
If I re-read this now, maybe this was a timing/race issue? Actually, I think the promises should hopefully be properly chained, so the scrolling is only triggered after the text is set.

I.e. when you use too much text this may just be the origin issue: The generation takes too long and the text selection/scrolling correction is already triggered.

@Rad0n So did you also test the bug as described above? Or in another ways?

[pradyumnamahajan]

Yes it was a timing issue and now that i think of it, a promise will be even better.

What I have done is increased the TOP_SCROLL_TIMEOUT so that it will always have enough time to scroll to top(which was not happening with 10 ms)

I did test it with other long URLs and with "Automatically use text selected on website" on and it worked with both of them :)

[rugk]

that it will always have enough time

Bold statement 😜
"Always" statements in programming and when fixing timing issues are always very critical… 😜

But, if it works, it's all right… 😄

I did test it with other long URLs and with "Automatically use text selected on website" on and it worked with both of them :)

Okay, great so… 😄

[pradyumnamahajan]

Hahaha yeah poor choice of words :P
It worked well even with 15 ms for my machine so I set it to 20 ms but tell me if you want to be more cautious and set it higher.

[pradyumnamahajan]

@rugk any other changes you want me to make? would love to help :) #hacktober

[rugk]

You may have a look at all issues tagged with "hacktoberfest" in this repo or somewhere else. (Of course, however, you can take any issue on. "help wanted" is also a good task if you want to contribute something possibly more difficult, but really useful)