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

url being selected in new tab when open tabs via external links #12094

Closed
kjozwiak opened this issue Nov 24, 2017 · 10 comments
Closed

url being selected in new tab when open tabs via external links #12094

kjozwiak opened this issue Nov 24, 2017 · 10 comments
Labels
0.19.x issue first seen in 0.19.x addressed-with-brave-core Needs confirmation, but this issue may be resolved with Brave Core. cr63 feature/tabsbar priority/P5 Cosmetic. Spelling, copy, layout. New features (which should also be part of an initiative). QA/test-plan-specified regression wontfix

Comments

@kjozwiak
Copy link
Member

Description

When clicking on external links, the URL is being selected in the new tab while the page is being loaded.

Steps to Reproduce

  1. launch 0.19.101 which includes C63
  2. login into gmail.com
  3. open an email (GitHub) and click on an external link at the bottom of the email

Actual result:

URL is being selected in the new tab that's being opened.

urlbarselected

Expected result:

URL shouldn't be selected in the new tab when clicking on external links. In the latest 0.19.95 released build, the URL isn't being selected.

Reproduces how often:

100% reproducible using the above STR.

Brave Version

about:brave info:

Brave: 0.19.101
rev: 30c42b5
Muon: 4.5.18
libchromiumcontent: 63.0.3239.40

Reproducible on current live release:

Not reproducible with 0.19.95 which is the latest release build.

Additional Information

  • macOS 10.12.6 x64 - Reproduced
  • Win 10 Pro x64 - Reproduced
  • Ubuntu 17.10 x64 - Reproduced
@kjozwiak kjozwiak added this to the 0.19.x + C63 (Release Channel) milestone Nov 24, 2017
@srirambv
Copy link
Collaborator

Looks fixed on 0.19.102 on Windows. Needs to be verified other OS

@bsclifton
Copy link
Member

Is this related to #11796 ?

@bsclifton
Copy link
Member

Confirmed this works great on C62 using 0.19.x (basically, I reverted to Muon 4.5.16 and it works great)

This does appear to be C63 related
cc: @darkdh

@alexwykoff alexwykoff added the priority/P2 Crashes. Loss of data. Severe memory leak. label Dec 12, 2017
@bsclifton bsclifton self-assigned this Dec 13, 2017
@petemill
Copy link
Member

Here's what some investigation has uncovered:

Both before and with C63, the url bar does get focused and the selected text made active. Then after some time, the page loads and the webview steals focus away from the urlbar.

The reason why the pre-C63 version does not have its text selected during this phase, is that the url bar changes to the new tab, and focuses itself due to the state being set to be selected when a frame is created, whilst the url is set to blank, before the frame gets a url.

However, the C63 version always has a Url, from the time the urlbar is first switched to the new tab / frame. So when it calls focus() on itself, it does so with text, which gets selected.

We can keep digging and find out why the C63 version has a location property much earlier with the C63 version, and/or we can 'fix' the urlbar so that perhaps it does not auto-focus itself if there is a valid location string already specified - as I would imagine that we only want the urlbar to automatically focus itself when the address bar is blank? That may be naive as I'm sure there are many code-paths and scenarios where focus should be set to the url bar. However, it seems like it should be valid that the state for the urlbar has a location right from the time the tab is created.

However, I'd say the risk of breaking something else is fairly high as it's difficult to see all the intended code paths for urlbar focus and selection.

@petemill
Copy link
Member

Seems like the muon tab event 'did-finish-navigation' is firing a lot earlier with the C63 version than previously. As above, this shouldn't be a problem neccessarily (though that should be investigated), but it is a race condition in b-l urlbar rendering that is causing the Url to be selected.

@bsclifton
Copy link
Member

Thanks for the investigation, @petemill 😄 👍

The issue described in the original post is definitely not happening anymore (I also verified and @srirambv reports above). When testing, I used the following URL which opens a new tab using target="_blank"
https://codepen.io/scheibo/pen/rWvQYK

The URL behavior itself could be better- but I think we could tackle that with some refactoring 😄 Given the above, I'm going to close the issue as the original reported issue no longer reproduces

@kjozwiak
Copy link
Member Author

kjozwiak commented Dec 13, 2017

@bsclifton this is still reproducible with 0.19.115 using the case under #12094 (comment).

@bsclifton bsclifton modified the milestones: 0.19.x + C63 (Release Channel), Backlog (Prioritized) Dec 13, 2017
@bsclifton bsclifton added priority/P5 Cosmetic. Spelling, copy, layout. New features (which should also be part of an initiative). and removed priority/P2 Crashes. Loss of data. Severe memory leak. release-notes/exclude labels Dec 13, 2017
@bsclifton bsclifton reopened this Dec 13, 2017
@petemill
Copy link
Member

I see it too @kjozwiak and note that we were confused - the original description and STR does not mention that the focus is stolen permanently, but I also see it in very similarly in 0.19.106 / muon 4.5.16. The timing may be a bit different but the result is the same, on both 0.19.x and 0.19.x+C63, the url bar is focused before the new foreground tab is finished loading - would you agree? Sometimes this results in the URL being selected and focus returned to the loaded page, sometimes it results in something worse: the URL bar steals focus, but the URL bar is always focused (even without the text being selected).
Here's what I see on 106:
issue-urlbar-focused-selected-newtab

@bsclifton
Copy link
Member

After talking with @kjozwiak, I reset the priority of this to P5 since focus is no longer being stolen. I think we can treat this as the selection (and any delay) being the bug 😄

@petemill thanks for digging in and confirming that the focus was indeed stolen (and that the issue is fixed)

@bsclifton bsclifton removed the bug label Dec 13, 2017
@afita
Copy link

afita commented Jun 29, 2018

I get this behavior on Linux. New tabs opened from links have focus in the URL bar. It's very annoying.

Version information (from Help->About Brave):

Brave: 0.23.19
V8: 6.7.288.46
rev: 178c3fb
Muon: 7.1.3
OS Release: 4.15.0-13-generic
Update Channel: Release
OS Architecture: x64
OS Platform: Linux
Node.js: 7.9.0
Tor: 0.3.3.7 (git-035a35178c92da94)
Brave Sync: v1.4.2
libchromiumcontent: 67.0.3396.87

@bsclifton bsclifton added wontfix addressed-with-brave-core Needs confirmation, but this issue may be resolved with Brave Core. labels Sep 1, 2018
@bsclifton bsclifton removed this from the Backlog (Prioritized) milestone Sep 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
0.19.x issue first seen in 0.19.x addressed-with-brave-core Needs confirmation, but this issue may be resolved with Brave Core. cr63 feature/tabsbar priority/P5 Cosmetic. Spelling, copy, layout. New features (which should also be part of an initiative). QA/test-plan-specified regression wontfix
Projects
None yet
Development

No branches or pull requests

6 participants