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

Clicking link from email will sometimes will do a search instead of directly following #5911

Closed
bsclifton opened this issue Nov 29, 2016 · 4 comments · Fixed by #6086
Closed

Comments

@bsclifton
Copy link
Member

bsclifton commented Nov 29, 2016

Did you search for similar issues before submitting this one?
Yes

Describe the issue you encountered:
With the following conditions:

  • Brave being default browser
  • clicking email link (email marketing) in a native client such as Microsoft Outlook

Brave will sometimes launch and then immediately do a search using your default search engine. When I dug deeper, I noticed that a period at the end of the URL was triggering this. You could repro by pasting the full URL into the omnibox and hitting enter. Removing that single period fixed the issue

@bradleyrichter also saw this behavior and can provide an example. Having a different default browser open works as you'd expect.

Expected behavior:
Links like this should be followed, not causing a search.

  • Platform (Win7, 8, 10? macOS? Linux distro?):
    macOS

  • Brave Version:
    0.12.10

  • Steps to reproduce:

    1. Launch Brave and open a new tab
    2. Paste in https://e.turbotax.intuit.com/pub/cc?_ri_=3vv-8-e. and hit enter
    3. Notice a search is done using your default search engine
    4. Paste in https://e.turbotax.intuit.com/pub/cc?_ri_=3vv-8-e (notice: missing the period at the end) and hit enter
    5. Notice that Brave properly follows the link (in this case, the link doesn't go anywhere; I sanitized it so that folks can't re-sign me up for emails! 😄 )
  • Screenshot if needed:

  • Any related issues:

@bsclifton bsclifton added the shell-integration Integrating brave with OS-level UI features. label Nov 29, 2016
@bsclifton
Copy link
Member Author

cc: our URL bar experts 😄
@aekeus @bbondy @diracdeltas

@bsclifton
Copy link
Member Author

bsclifton commented Nov 29, 2016

I created a PR which has a failing test: #5912

If you grab this issue, please reopen/merge that PR OR cherry-pick 3f514d8 and run it to verify that issue is actually fixed 😄

@Sh1d0w
Copy link

Sh1d0w commented Nov 29, 2016

@bsclifton Possible places to check where it goes wrong:

https://github.com/brave/browser-laptop/blob/master/js/lib/urlutil.js#L108

https://github.com/brave/browser-laptop/blob/master/js/lib/urlutil.js#L110

https://github.com/brave/browser-laptop/blob/master/js/lib/urlutil.js#L78

It is because of one of those checks

@bsclifton
Copy link
Member Author

bsclifton commented Dec 7, 2016

Bumped up milestone / assignee to match #6058 (I had closed it as a dupe)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants