Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Autocomplete does not work well #3049

Closed
ezyang opened this issue Aug 9, 2016 · 12 comments
Closed

Autocomplete does not work well #3049

ezyang opened this issue Aug 9, 2016 · 12 comments
Assignees
Labels
feature/autocomplete parity Features which should be supported in Brave since they're supported in other major browsers. QA/checked-Linux QA/checked-macOS QA/checked-Win64
Milestone

Comments

@ezyang
Copy link

ezyang commented Aug 9, 2016

Did you search for similar issues before submitting this one? I browsed the issues labeled autocomplete. #883 is sort of related but there are more issues than just described in that ticket.

Describe the issue you encountered: So, in general, I find brave's autocomplete to be far less useful than Firefox's/Chrome's. This is a bit hard to quantify, but let me describe some concrete problems which I have noticed:

  1. Brave only ever seems to offer two autocomplete suggestions from history. This is too few. Chrome by default offers five; Firefox offers ten. This is far more useful!
  2. Brave doesn't seem to update the autocomplete list based on frequency of access. For example, I access the URL https://github.com/ezyang/cabal less frequently than https://github.com/haskell/cabal but somehow I have poisoned my cache so that it only offers an autocomplete to ezyang (frustratingly, due to (1), I only ever get two autocompletes, both of which are ezyang! If Brave showed enough completes that I could find the one I wanted, this would be less of an issue.)
  3. Brave keeps forgetting my history. I notice this because after a while, history autocompletes that used to be provided disappear (not sure if this is correlated with closing the browser). I can't tell if this is just because Brave doesn't really keep history, for some reason my "Show All History" button has always been greyed out. I didn't see any tickets describing this, so I assumed it was just something not implemented on Linux.
  • Platform (Win7, 8, 10? macOS? Linux distro?): Ubuntu Linux Xenial, running Xmonad window manager
  • Brave Version: 0.10.4
@bradleyrichter
Copy link
Contributor

@ezyang Thanks for submitting this request. We are filling in many of the missing history features for our 1.0 launch so this should improve.

We can certainly add at least 2 more entrees for autocomplete matches which will also help solve this issue.

cc @bbondy

@bsclifton bsclifton added enhancement parity Features which should be supported in Brave since they're supported in other major browsers. labels Aug 11, 2016
@bsclifton
Copy link
Member

bsclifton commented Aug 11, 2016

I too have this issue... I have a great example which I can share

I will sometimes go to https://google.com/analytics or similar. When I type in google.com, it' auto-completes that entire URL instead of preferring the shorter google.com 😦

Also, I know you can delete auto-complete entries, but I don't remember how (it's a keyboard shortcut only, if I remember right)

@bradleyrichter
Copy link
Contributor

@alexwykoff The above reports are from Linux so let's confirm this behavior is tri-platform. I think it is.

@bradleyrichter
Copy link
Contributor

funny...apparently FX needs some work too:

(typing mens hair and getting mens shoes?)
image

@Sh1d0w
Copy link

Sh1d0w commented Sep 6, 2016

@bradleyrichter These are search suggestions from your default search engine. It is up to it what it will return as search results so this is no a browser issue.

The bug that I reported for Brave can be clearly seen in the screen cast I have provided. The suggested url is by Brave's autocomplete rather than from the search engine.

Try repeating the same steps as mine under Chrome or FF. Both will suggest the mosy recent or most visited website as highlighted text, but Brave doesn't.

@bradleyrichter
Copy link
Contributor

@Sh1d0w not sure why google is giving FX those strange results but chrome and brave are fine with the same steps.

But anyway - your bug above is duly noted and certainly a different issue. We will work on it. Thanks...

@alexwykoff
Copy link
Contributor

Win 7 : 0 results
autocomplete_win7

Ubuntu : 3 results
autocomplete_ubuntu

OS X : 0 results
autocomplete_osx

@aekeus aekeus self-assigned this Sep 27, 2016
@aekeus aekeus added this to the 1.0.0 milestone Sep 27, 2016
@aekeus aekeus modified the milestones: 0.12.6dev, 1.0.0 Oct 11, 2016
aekeus added a commit that referenced this issue Oct 12, 2016
…ount

  * Re-order results to show history items before bookmark items
  * Record number of times site is visited
  * Re-order history items based on number of times visited and last access timestamp
  * Update and create tests

Fixes: #3049

Auditors: @bbondy

Test plan:

A new attribute, count, is added to history items in the sites section of save-session-1. This attribute records the number of times a site has been accessed. The test plan is broken in two components, one to verify the count attribute is incremented on access, the second to verify that the count and last access time are used when sorting history suggestions.

Scenario 1

Part A

  1. Clear save-session-1
  2. Access a site
  3. Close browser
  4. Verify that the count attribute for the saved site in save-session-1 has a value of 1

Part B

  1. Open browser
  2. Navigate to site from Part A
  3. Close browser
  4. Verify that the count attribute for the saved site in save-session-1 has an incremented value

Scenario 2

  1. Clear save-session-1
  2. Open browser
  3. Visit gm.com and gm.ca
  4. Close browser
  5. Verify that the count attributes of the two new sites are identical (1)
  6. Open browser to a new tab
  7. Enter ‘gm’ into the url bar
  8. Verify that the most recently visited of the two gm sites is the first entry in the history auto-suggest list
aekeus added a commit that referenced this issue Oct 13, 2016
…ount

  * Re-order results to show history items before bookmark items
  * Record number of times site is visited
  * Re-order history items based on number of times visited and last access timestamp
  * Update and create tests

Fixes: #3049

Auditors: @bbondy

Test plan:

A new attribute, count, is added to history items in the sites section of save-session-1. This attribute records the number of times a site has been accessed. The test plan is broken in two components, one to verify the count attribute is incremented on access, the second to verify that the count and last access time are used when sorting history suggestions.

Scenario 1

Part A

  1. Clear save-session-1
  2. Access a site
  3. Close browser
  4. Verify that the count attribute for the saved site in save-session-1 has a value of 1

Part B

  1. Open browser
  2. Navigate to site from Part A
  3. Close browser
  4. Verify that the count attribute for the saved site in save-session-1 has an incremented value

Scenario 2

  1. Clear save-session-1
  2. Open browser
  3. Visit gm.com and gm.ca
  4. Close browser
  5. Verify that the count attributes of the two new sites are identical (1)
  6. Open browser to a new tab
  7. Enter ‘gm’ into the url bar
  8. Verify that the most recently visited of the two gm sites is the first entry in the history auto-suggest list
aekeus added a commit that referenced this issue Oct 15, 2016
…ount

      * Re-order results to show history items before bookmark items
      * Record number of times site is visited
      * Re-order history items based on number of times visited and last access timestamp
      * Update and create tests

    Fixes: #3049

    Auditors: @bbondy

    Test plan:

    A new attribute, count, is added to history items in the sites section of save-session-1. This attribute records the number of times a site has been accessed. The test plan is broken in two components, one to verify the count attribute is incremented on access, the second to verify that the count and last access time are used when sorting history suggestions.

    Scenario 1

    Part A

      1. Clear save-session-1
      2. Access a site
      3. Close browser
      4. Verify that the count attribute for the saved site in save-session-1 has a value of 1

    Part B

      1. Open browser
      2. Navigate to site from Part A
      3. Close browser
      4. Verify that the count attribute for the saved site in save-session-1 has an incremented value

    Scenario 2

      1. Clear save-session-1
      2. Open browser
      3. Visit gm.com and gm.ca
      4. Close browser
      5. Verify that the count attributes of the two new sites are identical (1)
      6. Open browser to a new tab
      7. Enter ‘gm’ into the url bar
      8. Verify that the most recently visited of the two gm sites is the first entry in the history auto-suggest list
aekeus added a commit that referenced this issue Oct 17, 2016
…ount

      * Re-order results to show history items before bookmark items
      * Record number of times site is visited
      * Re-order history items based on number of times visited and last access timestamp
      * Update and create tests

    Fixes: #3049

    Auditors: @bbondy

    Test plan:

    A new attribute, count, is added to history items in the sites section of save-session-1. This attribute records the number of times a site has been accessed. The test plan is broken in two components, one to verify the count attribute is incremented on access, the second to verify that the count and last access time are used when sorting history suggestions.

    Scenario 1

    Part A

      1. Clear save-session-1
      2. Access a site
      3. Close browser
      4. Verify that the count attribute for the saved site in save-session-1 has a value of 1

    Part B

      1. Open browser
      2. Navigate to site from Part A
      3. Close browser
      4. Verify that the count attribute for the saved site in save-session-1 has an incremented value

    Scenario 2

      1. Clear save-session-1
      2. Open browser
      3. Visit gm.com and gm.ca
      4. Close browser
      5. Verify that the count attributes of the two new sites are identical (1)
      6. Open browser to a new tab
      7. Enter ‘gm’ into the url bar
      8. Verify that the most recently visited of the two gm sites is the first entry in the history auto-suggest list
@Sh1d0w
Copy link

Sh1d0w commented Oct 19, 2016

This does not seem to work for me. I've built it from master 497ecf8 and installed it on my system.

I've did the following:

  1. Typed tw in the url bar
  2. Those two entries appeared under History autocomplete in the following order: https://www.twilio.com/ and https://www.twitter.com
  3. I choosed https://www.twitter.com and visited the website
  4. In new tab I repeated the same but the order of the websites did not changed.
  5. I repeated steps 1-3 several times resulting no change even after browser restart

Should I delete all brave related session files in order that to work? It should altomatically work after updating Brave to newest version without requiring any further steps.

Please close if I did something wrong and this report is not valid.

@Sh1d0w Sh1d0w reopened this Oct 19, 2016
@aekeus
Copy link
Member

aekeus commented Oct 19, 2016

The ordering of the websites is controlled by a count attribute that is stored with each history item. The history items will be ordered by the count (highest first) with some decay based on the last time the website is accessed.

I just tested with a new session file, accessing twilio.com and then twitter.com. Each history item has a count of one.

screen shot 2016-10-19 at 8 55 09 am

Twitter is the first entry in the list as it was accessed most recently.

Thanks for the test. Please let us know if you are seeing something different than what I described.

@aekeus aekeus closed this as completed Oct 19, 2016
@Sh1d0w
Copy link

Sh1d0w commented Oct 19, 2016

@aekeus I understand this and it is working just perfectly if I remove session-store-1.

The problem is the following:

When we update Brave, the session store file is not removed. It stays as it is, and when you are accessing websites count entry is not created under the visited entry, as it is already in the session file.

To sum it up - everything from your PR is working if I delete session-store-1 file, but you can't expect the users to delete it after the upgrade as they will loose all of their settings and history. The issue is that if you already have session-store-1 file, your PR is not working since the count entry is not appended to the already existing history entries, only to new ones.

For example with the PoC I provided in the comment before yours, when I inspect session-store-1 file the twitter.com entry does not have count property no matter how much times I access this website, because I have visited it before. If I visit webite that I have never visited, it correctly has the count: 0 property.

Sorry for reopening this again or if I haven't explained it well the first time. Let me know if you need more info on this issue.

@Sh1d0w Sh1d0w reopened this Oct 19, 2016
@aekeus
Copy link
Member

aekeus commented Oct 19, 2016

Hi, Thanks for checking this for us again. I will try and reproduce and fix.

@aekeus
Copy link
Member

aekeus commented Oct 19, 2016

Ok, I can reproduce the issue you are seeing although the root cause is different.

The count attribute is added to existing entries in an existing session file. I confirmed this multiple times by removing the count attribute, opening the browser, navigating to a site, closing the browser and inspecting the session file.

In the specific instance you noticed above the history item have the following locations:

When you enter the characters twi the locations match on the first portion of twitter.com but later in www.twilio.com. twitter.com will always show up first regardless of the number of times you navigate to www.twilio.com.

I am going to close this issue and open another that specifically fixes the prefix issue described above.

Thanks again for looking into this for us.

cc @bbondy

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature/autocomplete parity Features which should be supported in Brave since they're supported in other major browsers. QA/checked-Linux QA/checked-macOS QA/checked-Win64
Projects
None yet
Development

No branches or pull requests

7 participants