1694105 - Style page action buttons as chiclets

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, enhancement, P2)

Description

[Cristian Fogel [:cfogel]]

Affected versions

• 87.0a1(2020-02-21)

Affected platforms

• Windows 10;

Steps to reproduce
proton prefs set on true
browser.proton.toolbar.version 2

1. Launch Firefox, access any page;
2. Click on the Bookmark button;

Expected result

• no issues;

Actual result

• additional top-bottom space for buttons inside address bar;

Regression range

• First bad: 2021-02-18;
• Last good: 2021-02-17;
• Pushlog: URL
• Potential regressor: bug 1686530

Additional notes

• linked screenshot here - MoCO only;
• added to the current component due to "regressor" even though the impact is over the Address Bar .
• issue not visible on macOS(10.15).

Comment 1

[:Gijs (he/him)]

Mark, can you take a look, esp. as to whether this affects non-proton and/or whether this is something we need to fix before handoff/

Comment 2

[Mark Striemer [:mstriemer]]

I can't reproduce this without the proton toolbar enabled. This is regressed by the URL bar height increasing, I believe. These buttons are set to 28px tall and there's now 30px of space in their parent.

Since these are technically in the address bar component they aren't part of the toolbar hand-off so this isn't critical for that.

It would be nice to fix this, but I think this rule is actually what we want. The buttons should stay as 28px tall and it looks like the address bar will lose its border and that 1px gap that looks like a bug will result in a nice 2px gap giving us a "chicklet".

Comment 3

[Mark Striemer [:mstriemer]]

Since this is sort of interim until we get the page action buttons styled, I'm going to rebrand this bug as an enhancement, rather than filing one to dupe to :)

Comment 4

[Julien Cristau [:jcristau]]

I think the bot won't like this being flagged as enhancement and a regression at the same time.

Comment 5

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Bug 1691545 may have fixed, this, I still have a few tweaks in bug 1696628 that may further improve it. So we can check after that.

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Needinfo to mak to make sure we don't forget to check now that those other bugs are fixed.

Comment 7

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

thanks, I'll just take the bug and check it's done.

Comment 8

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: Screenshot (126).png

The spacing is correct with browser.proton.urlbar.enabled (in addition to the main proton and the toolbar pref), the button is exactly 28px, and has the same border as the urlbar, and any other toolbarbutton. There should be a 2px padding all around it when using the default Proton themes, 1px of padding if the theme has a 1px border on the urlbar.

Of course at higher dpis, like 125%, due to roundings, the padding may not look even.

The buttons have both :hover and :hover:active

(note: the screenshot includes fixes to the urlbar Proton Dark theme that are about to land, but didn't land yet, from bug 1692046)

Comment 9

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

This is done, afaict.