Beaker: URL autocomplete breaks if you type too fast

[taravancil]

@SaFrMo implemented #803 which is amazing! I've been wishing for that feature for a quite a while.

I just rebuilt beaker-0.8 and found that it works almost perfectly. It gets a bit buggy if you type into the URL bar too quickly. I haven't dug in yet to fix it myself, but if I do, I'll post here so I don't duplicate anyone else's efforts.

[SaFrMo]

Good find, thanks! I'll try to take a look soon as well and will make any notes here if do.

[SaFrMo]

I tried building beaker-0.8 and wasn't able to replicate the issue, but looking at the code I added, there's a pretty big possible point of failure in the async search results here. @taravancil, if you have a more extensive history saved in Beaker than I do, that could be a pretty straightforward cause behind the bug!

One possible solution could be to reset a timeout after every keypress in the address bar - when that timeout finishes is when it actually executes the search. Another solution could be to disregard any existing promises on every keypress in the address bar, which would keep things nice and responsive.

I'll try to implement the second solution next week when I have some free time, unless there are other thoughts on causes or fixes!

[hossameldeen]

Since the callback here is passed to then(...), it'll be executed asynchronously. Is there a reason or need for that or can we just call it synchronously like that:

    beaker.history.search(value)
      .then(results => {
        handleAutocompleteSearch(results)
        if (autocompleteCurrentSelection !== -1 && autocompleteSuggestion) {
          // find the length of the current value
          var startingIndex = e.target.value.length
          // add the autocomplete suggestion
          e.target.value += autocompleteSuggestion
          // select the autocomplete suggestion
          e.target.setSelectionRange(startingIndex, e.target.value.length)
        }
      })

Or even just move it to handleAutocompleteSearch(...) if it makes sense to do so. Not familiar enough with this feature to tell.

---

But yes, this fix would just fix this issue. Aside from this specific issue and even before the PR, I think the code may override new results with older ones.

I have a third solution beside the two @SaFrMo suggested. Something like debouncing but instead of being based on a timeout, will be based on a lock. Will write the details if this fix isn't enough or we want to solve the overriding problem in this issue as well.

[taravancil]

We decided to take this feature out of beaker-0.8 for now 😢

Paul fixed the bug, but there was still an open question about performance and how to avoid the jittery re-rendering of the blue autocomplete part of the URL. I'm going to close this, but feel free to keep the discussion going.

[pfrazee]

I tried the solutions you suggested @hossameldeen, including a debounce with a 1ms timeout (which is close to a lock concept). That solved the issue, but the rendering performance was unacceptable.

When you look at how it works in chrome, it's extremely smooth. The suggested text is appended and highlighted with no flash.

For us, even without the debounce (where it worked on my computer) there was still a flash. The problem is (AFAIK) there's no way to specify a selected range in the HTML at the same time that we modify the value, which leads to a loop of render text, select region. That causes a flashing effect which didn't look good enough IMO.

We decided to remove it from the build. Sorry @SaFrMo, it was good work otherwise. If anybody can make a version that doesnt have flashing URL bars, let me know, but I'd be surprised.

[SaFrMo]

Makes sense! Thanks for the work, all - it's a shame that it's not going to work out, but it's definitely important to keep things smooth 😀

[Butterfly]

Hi! Just poking my (dog's) nose in.. If I'm correctly understanding "flashing" to mean "blinking", you're also talking about an accessibility accommodation point. That would be about how photosensitive epilepsy can be triggered by unexpected flashing (Smithsonian Magazine, Newsweek).

Informational links about flashing effects and the Web:

• Epilepsy Action highlighting W3C guidelines
• W3C Understanding Guideline 2.3: Seizures
• W3C Understanding Success Criterion 2.3.1: Three Flashes or Below Threshold
• W3C G15: Using flash rate test tool

Where flashing is unavoidable, e.g. video of lightning, the recommendation is to slow the flash rate down.

Anyway, am taking the long route around to say #ThankYou(!), that it's a good decision for #Life affecting reasons far beyond "just" aesthetics.. :smile:

[SaFrMo]

Great note @Butterfly ! That's an important point for sure, and those are super useful links. Thanks!

[hossameldeen]

I'm so excited! I think I've found a solution for the blinking problem... but it may complicate things.

I think the blinking problem might be due to the selection/autocomplete being replaced by the key the user presses, then rendered, then being added again, then rendered. So, if we listen to keypress event, call e.preventDefault(), and handle the keypress ourselves, the middle render won't happen & the blink doesn't occur anymore. At least, that's what seems to me when I tried it here.

The complication I fear is in handling the keypress our own. Don't know how complicated that may be.

Note: honestly, haven't tried the feature yet, so please excuse me if I'm mistaken in describing what happens in the blinking problem.

[SaFrMo]

@hossameldeen That's a really interesting idea and it looks great in the demo! If the blinking is the only issue, that might just be the fix - and according to this StackOverflow answer it's not that hard to mimic some parts of native keypress handling in JS.

I got part of the way through sketching out a fix here (which uses a slightly modified version of the SO answer here) and it's an encouraging start - it's simple enough to preventDefault any non-meta key value and store the results.

That's all I have time for at the moment to dig into the issue, but I'm excited to see if it's still a possibility! Definitely a really useful feature in other browsers and one I'd love in Beaker, so I can jump back in early next week and look around some more if that'd be helpful.

[pfrazee]

That definitely is promising.

On Thu, Jan 11, 2018 at 8:32 AM, Sander Moolin [e-mail]
wrote:

@hossameldeen https://github.com/hossameldeen That's a really
interesting idea and it looks great in the demo! If the blinking is the
only issue, that might just be the fix - and according to this
StackOverflow answer https://stackoverflow.com/a/4180715 it's not that
hard to mimic some parts of native keypress handling in JS.

I got part of the way through sketching out a fix here
https://github.com/SaFrMo/beaker (which uses a slightly modified
version of the SO answer here
https://github.com/SaFrMo/beaker/commit/eb4d8814090fd33534ef7184ce168ca887022431#diff-a896a04ea4e04c46471aab29a38dc2ceR742)
and it's an encouraging start - it's simple enough to preventDefault any
non-meta key value and store the results.

That's all I have time for at the moment to dig into the issue, but I'm
excited to see if it's still a possibility! Definitely a really useful
feature in other browsers and one I'd love in Beaker, so I can jump back in
early next week and look around some more if that'd be helpful.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/beakerbrowser/beaker/issues/807#issuecomment-356950798,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABNhUwvNud_sffJP8r_6rHm5PggOUU3sks5tJht9gaJpZM4RTzCg
.