1550584 - The space around the identity box separator/divider is now asymmetrical

Status: RESOLVED WORKSFORME

(Firefox :: Address Bar, defect, P1)

Description

[Drew Willcoxon :adw]

See bug 1548198, attachment 9063863 [details].

Comment 1

[Mehmet]

Probably interesting:

Issue is to see at:

https://www.apple.com/ (10px_divider_8px)

Issue is not to see at:

about:config (8px_divider_8px) or https://www.sparkasse.de/ (9px_divider_9px) or https://www.bankofamerica.com/ (9px_divider_9px)

Comment 2

[Drew Willcoxon :adw]

(In reply to Mehmet from comment #1)

https://www.bankofamerica.com/ (9px_divider_9px)

I actually see the problem here... :-/ Might be due to subsampling since I'm on a 2x screen and I think you said you're not.

Starting to wish I didn't fix the other bug the way I did.

Comment 3

[Drew Willcoxon :adw]

I tested the four URLs you mentioned with and without the patch (actually, just without the important .urlbar-input { padding: 0; } part) at 2x (layout.css.devPixelsPerPx = -1) and 1x (layout.css.devPixelsPerPx = 1) [1].

The summary is that even before the patch, (1) the spacing sometimes was off, (2) it was off nearly the same number of times it's off with the patch (without the patch, it's actually off one more time), and (3) it was off in the other direction.

I don't know why it can't be perfect regardless of the text. I'd like to find out. But since the patch didn't make it worse imo, just different, I'm going to close this. I'm sure Dao will reopen it if he disagrees.

If you happen to run the same test and get a different summary, please let me know, or even if you just compare your build before the patch and after without checking 1x vs 2x. It looks like we did get different results based on your comment 1, but in that case I'd expect to get different results elsewhere and the overall summary would be the same. If not, that would be interesting.

[1] Results:

before (without .urlbar-input { padding: 0; })
about:config
1x: bad 8px vs ~10px
2x: bad ~17px vs 19px
https://www.bankofamerica.com/
1x: bad 9px vs 10px
2x: good 19px vs 19px
https://www.apple.com/
1x: bad 9px vs 10px
2x: good 19px vs 19px
https://www.sparkasse.de/
1x: good ~9px vs 9px
2x: bad 18px vs 20px

after (with .urlbar-input { padding: 0; })
about:config
1x: bad 8px vs 9px
2x: good ~17px vs 17px
https://www.bankofamerica.com/
1x: good 9px vx 9px
2x: bad 19px vs 17px
https://www.apple.com/
1x: good 9px vx 9px
2x: bad 19px vs 17px
https://www.sparkasse.de/
1x: bad ~9px vs 8px
2x: good 18px vs 18px

Comment 4

[Drew Willcoxon :adw]

I just ran a Windows 10 VM at 1x -- the VM itself, not by setting layout.css.devPixelsPerPx = 1 -- and as expected, there was 1px of difference in spacing between the 2019-05-09-21-43-05 nightly (with the patch) and the 2019-05-06-21-46-17 nightly (without the patch). The one with the patch had symmetrical spacing, and the one without had 1px extra on the right-hand side.

I verified the 2019-05-09-21-43-05 nightly (https://hg.mozilla.org/mozilla-central/rev/f28f80e2466839e5b50c9b9f728988044fecb186) includes the patch by grepping the shortlog (https://hg.mozilla.org/mozilla-central/shortlog/f28f80e2466839e5b50c9b9f728988044fecb186), and by using the browser toolbox to check that .urlbar-input's padding is 0. I also verified that the padding in the 2019-05-06-21-46-17 nightly is 1px.

Comment 5

[Drew Willcoxon :adw]

(In reply to Drew Willcoxon :adw from comment #4)

The one with the patch had symmetrical spacing

Although I should say it's sometimes unclear what to count as space and what not, due to subsampling. Regardless, in all my tests, there was clearly one extra column of space on the right-hand side before the patch.