extend blue outline in URL bar to include bookmark button

[jkup]

• Submitted a ticket for my issue if one did not already exist.
• Used Github auto-closing keywords in the commit message.
• Added/updated tests for this change (for new code or code which already has tests).
• Ran git rebase -i to squash commits (if needed).

Fix #5439

Auditors @bsclifton @bradleyrichter

@bradleyrichter this ticket presents a little bit of a tough problem.

The bookmark star needs to have a gray background but also needs to be positioned over the white urlbar. The only problem is if the width and height of the bookmark star background are 100% of the urlbar then the glow won't show up.

What I've done so far is made the grey background 1px smaller than the urlbar on all sides so technically there is a tiny white space in between but I think it looks alright.

Here are some screenshots of what I've come up with so far: let me know what you think!

Test Plan:

1. Click in the urlbar
2. Make sure the blue focus ring covers the bookmark star
3. Make sure you can still click the bookmark star

[jkup]

Dang, for some reason the bookmark button isn't clickable on Windows. I'm trying to figure out if it's a positioning or maybe a z-index thing. Any thoughts @bsclifton?

[jkup]

Update: Still trying to get my Windows VM working so I can go back to figuring this out.

[bsclifton]

Finally got my VM up and running yesterday 😄 Will be checking this out tonite...

[bsclifton]

The problem is the margin-left on the urlbar icon (#navigator .urlbarIcon)

I had success by changing from margin (margin-left: 20px) to padding (padding-left: 20px). When I looked it up, it seems that margin is not intended to be clickable 😄

[jkup]

@bsclifton updated!

[bsclifton]

plusplus 😄

[bsclifton]

@jkup can you update original post with testing steps? Thanks!

[luixxiul]

@jkup

I'm trying to figure out if it's a positioning or maybe a z-index thing.

There are z-index variables inside variable.less to avoid a case like #1910 (PR #2074). I think the z-index you have set should be 2100 as the button is a subcomponent of the navigation bar, whose zindex is 2000. Please have a look at https://github.com/brave/browser-laptop/blob/master/less/variables.less#L124

Thanks!

[bradleyrichter]

@jkup did you see this on mac?

[jkup]

@luixxiul thanks for the tip! it ended up not being a z-index issue :)

@bradleyrichter what am I looking at here?

[bsclifton]

Removed milestone for now (since change was reverted)

@jkup there is a gap between the home button and the favicon star