1323001 - Location / search bar splitter active region is not as wide as it should be on Windows and Linux

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Dão Gottwald [:dao]]

Attachment: patch

The splitter seems to be behind the location bar, so it fails to capture mouse events there. I believe at some point this worked without position:relative, not sure how it broke. Or maybe this was just poorly tested and never worked.

Also on Windows the splitter doesn't overlap the textboxes' border whereas it kind of tried to do that on Linux (although this didn't work at all due to the above bug).

Finally I believe we don't need to set a height on Mac since we don't vertically center elements in the toolbar anymore.

Comment 1

[:Gijs (he/him)]

Comment on attachment 8818001 [details] [diff] [review]
patch

Review of attachment 8818001 [details] [diff] [review]:
-----------------------------------------------------------------

Please ensure that when you commit this change, the change summary does not imply (as the bug's summary does - the patch has no commit message, it seems) that its changes will only affect Windows.

::: browser/themes/linux/browser.css
@@ -781,5 @@
> -  margin-inline-start: -4px;
> -}
> -
> -#urlbar-search-splitter + #search-container > #searchbar > .searchbar-textbox {
> -  margin-inline-start: 0;

This change visually moves the url bar and search bar to be further apart. Was that intentional? I don't see it mentioned in your comment / bug summary.

Comment 2

[Dão Gottwald [:dao]]

(In reply to :Gijs Kruitbosch from comment #1)
> Please ensure that when you commit this change, the change summary does not
> imply (as the bug's summary does - the patch has no commit message, it
> seems) that its changes will only affect Windows.

The summary actually says Windows and Linux. The height removal on Mac was really just a drive-by fix...

> ::: browser/themes/linux/browser.css
> @@ -781,5 @@
> > -  margin-inline-start: -4px;
> > -}
> > -
> > -#urlbar-search-splitter + #search-container > #searchbar > .searchbar-textbox {
> > -  margin-inline-start: 0;
> 
> This change visually moves the url bar and search bar to be further apart.
> Was that intentional? I don't see it mentioned in your comment / bug summary.

As far as I can tell it actually moves them closer together by 1px since the splitter width is and was 8px, half of which we'll compensate with a -4px negative margin-inline-end, but .searchbar-textbox's margin-inline-start is 3px. This is intentional. The space between the two textboxes should now be 6px as implied by their margins.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/52e782555b4b
Make the location / search bar splitter's active region span the two textboxes' combined neighboring margin and border width on Windows and Linux. r=gijs
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ed5a7be683a
Stop setting a height on the location / search bar splitter since we don't vertically center elements in the toolbar anymore. r=gijs

Comment 4

[Dão Gottwald [:dao]]

(In reply to Pulsebot from comment #3)
> Pushed by [e-mail]:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/52e782555b4b
> Make the location / search bar splitter's active region span the two
> textboxes' combined neighboring margin and border width on Windows and
> Linux. r=gijs

Backed out: https://hg.mozilla.org/integration/mozilla-inbound/rev/a2383f4f7efe313746ad1dd16bfe9ab9c7db1e36

Hitting the same failures as I did in bug 1322430 because the search bar disappeared from the toolbar.

https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=8ed5a7be683af8322e7bc21df732aad283a3e59a&selectedJob=40524593

https://public-artifacts.taskcluster.net/N6vyQylOTh-GkOTHqoIYFA/0/public/test_info//mozilla-test-fail-screenshot_fq8Mzk.png

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0c650d7f2609
Make the location / search bar splitter's active region span the two textboxes' combined neighboring margin and border width on Windows and Linux. r=gijs

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/8ed5a7be683a
https://hg.mozilla.org/mozilla-central/rev/0c650d7f2609