1901904 - New-tab pocket stories' 3-dot-menu-icon is misaligned in its button (0.5px-1px to the right of the perfect center position)

Status: RESOLVED FIXED

(Firefox :: New Tab Page, defect, P3)

Description

[Daniel Holbert [:dholbert]]

I noticed today that the "..." icon at the top-right of each Pocket story on about:newtab is positioned just a little bit to the right of center within that button.

At a default 100% display-pixel-ratio, this isn't super noticeable since there's only one more pixel of space on the left side as compared to the right side. But at a 125% display-pixel-ratio (the default on many Windows systems), the rightward-bias gets emphasized and we end up with two extra pixels of space on the left as compared to the right.

See attached screenshot.

The reason for this is pretty simple - we're positioning this image (a background image) using backward-position:55% on the .context-menu-button element. 55% here means slightly-to-the-right of center. If I remove that declaration and let an (otherwise-overridden) backward-position: center center rule take effect, then the icon looks centered.

Is there any reason we're using background-position:55% here, or that we want it slightly-to-the-right of center? The barely-uncentered-ness feels unsettling to me right now and I'm guessing it's inadvertent and was originally some sort of hackaround or mistake...

Comment 1

[Daniel Holbert [:dholbert]]

STR:

1. Set a 125% display pixel scaling ratio in your OS settings
   OR, set a 100% pixel scaling ratio, and use Ctrl+Mousewheel to zoom in to 130% (Ctrl + mousewheel for two ticks -- don't use Ctrl and +, which doesn't let you traverse to 130%)

2. Look very closely at the 3-dot menu that appears when you hover new-tab pocket story. Use a zoom tool or take a screenshot and measure how many pixels there are inside the button, to the left and right of the dots.

ACTUAL RESULTS:
9 pixels to the left of the dots, vs. 7 pixels to the right of the dots. So the left blank area is 2px wider than the right blank area. Or put another way, we're 1px off-center.

EXPECTED RESULTS:
They should draw with 8px to the left and 8px to the right, in order to be centered in the button.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: screenshot of 3-dot-menu button on a pocket story when viewing newtab at 130% full-page zoom

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: 10x zoomed in version of previous screenshot

Here's that screenshot again, zoomed in 10x to make the imbalance easier to see. (I added text to annotate the 9px vs 7px distance to the button sides, too.)

Comment 4

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #0)

The reason for this is pretty simple - we're positioning this image (a background image) using backward-position:55% on the .context-menu-button element.

Source links:
https://searchfox.org/mozilla-central/search?q=background-position%3A+55%25&path=&case=false&regexp=false

(There are only 7 matches for that declaration -- I think one of the matches in each activity-stream-{platform} are the relevant declarations here.)

Comment 5

[Daniel Holbert [:dholbert]]

Digging backwards through blame, I'm not seeing any clear reason why we used 55% (which is good news for this being something we can change).

In particular, it looks like this background-position:55% came from a merge from the activity-stream github repo; and over there, it dates back to https://github.com/mozilla/activity-stream/commit/499f955d5af5e2a7fff2d9a4c536a2349ac9d098 which was a large iconography update (and before that, it looks like we had background-position: 65% which would've been even more off-center; I'm guessing maybe that was before we started drawing the button's border, so the off-centered-ness wasn't noticeable, perhaps?)

Anyway: all of that is to say, it doesn't look like there's any strong reason to treat this 55% value as especially important here.

We could just remove this declaration entirely to get as-close-to-center-as-possible at all zoom levels -- but instead of that, I'd suggest we reduce it to 50.1% -- that way, if the exact center location is on a pixel boundary, we'll choose to center the image just-to-the-right of that pixel rather than potentially arbitrarily aligning just-to-the-left-of-it (which I think was probably our intent when choosing 55%). This will make the icon remain very-slightly-rightwards-biased when we are forced to nudge it off-center (e.g. in cases where we have 6px of space on one side and 7px of space on the other side, etc); but it won't make us nudge the icon off-center by 2px as described in comment 1. At least, not for reasonable button-sizes / scale-factors.

The button here is 27 CSS pixels wide, so a very-slight 0.1% rightwards-bias (from 50.1%) would be 0.001*27 = 0.027 CSS pixels, which is conveniently:

• just greater than a "nscoord" layout app unit (which is 1/60th of a CSS pixel = 0.016 CSS pixels), i.e. it's large enough to be meaningful as an extremely tiny nudge.
• smaller than 1 display pixel for devicePixelRatios <= 37 (which is pretty extreme; actual devicePixelRatios are on the order of 1 to 3, or maybe 15 in extreme cases with 300% pixel-scaling and 500% full-page-zoom simultaneously applied).
• So it won't bias us rightwards by more than 1 pixel.

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: Bug 1901904: Improve centering for the newtab 3-dot-menu icon within its button. r?#home-newtab-reviewers

Previously we had a background-position of 55%, which is sorta close to center,
but far enough away from center that it placed the image a full pixel away from
the perfect center position at certain zoom or HiDPI levels, which looked
awkward (e.g. with 9px of space to the left of the image, and 7px of space to
the right of it, when perfect-centering would have 8px on either side).

I suspect the reason we were using 55% (rather than 'center') was to be a
"tiebreaker" in favor of a slight rightwards bias when there's an odd number of
extra pixels to use for spacing -- in that case, biasing to the right makes
sense (Npx on the left, N+1px on the right). The new background-position value
of 50.1% should still give us that behavior, without having the unwanted side
effect of nudging us off of the perfectly-centered position in cases where we
could put the same number of pixels on either side.

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: "After patch" screenshot of Firefox new-tab page at 130% zoom level with a pocket tile hovered

Comment 8

[Daniel Holbert [:dholbert]]

Attachment: "Before patch" screenshot of Firefox new-tab page at 130% zoom level with a pocket tile hovered

Comment 9

[Daniel Holbert [:dholbert]]

Attachment: "After patch" screenshot of Firefox new-tab page at 100% zoom level with a pocket tile hovered

Comment 10

[Daniel Holbert [:dholbert]]

Attachment: "Before patch" screenshot of Firefox new-tab page at 100% zoom level with a pocket tile hovered

Comment 11

[Daniel Holbert [:dholbert]]

Screenshot comparison:

• At 130% zoom level: the "after patch" screenshot has the icon nicely centered, while the "before patch" screenshot does not (icon is 1px to the right of center, with the left padding being 2px larger than the right padding).

• At 100% zoom level: the icon's rendering/placement is identical in the before & after screenshots. The icon can't be centered perfectly, so it's snapped just to the right of center (with 7px of padding on the left, 6px on the right), "breaking the tie" in favor of right-alignment.

(The 100% zoom level screenshots are provided just to demonstrate that there's zero behavior change there, not even e.g. antialiasing the image differently or anything like that.)

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c38b396e3bee
Improve centering for the newtab 3-dot-menu icon within its button. r=home-newtab-reviewers,maxx

Comment 13

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/c38b396e3bee