1666252 - What's new - focus ring overlaps and cuts dates on tab-select

Status: VERIFIED FIXED

(Firefox :: Messaging System, defect, P1)

Description

[Cristian Fogel, QA [:cfogel]]

Attachment: Untitled.png

Affected versions

• 82.0a1(2020-09-20);

Affected platforms

• Windows 10, macOS 11.0;

Steps to reproduce

1. Launch Firefox, click on the Hamburger menu;
2. Click on the What's new option;
3. Press the Tab key a couple of times;

Expected result

• no issues;

Actual result

• tab-selection indicator cuts the date for listed entries;

Regression range

• First bad build_date: 2020-05-10;
• Last good build_date: 2020-05-11;
• Potential regressor: 1630907

Additional notes

• attached screenshot with the issue;
• S3 as suggested severity.

Comment 1

[Emily McMinn :emcminn]

Attachment: Bug 1666252 - Margin adjustment for WNPanel

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4736c8579ee7
Margin adjustment for WNPanel r=andreio

Comment 3

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/4736c8579ee7

Comment 4

[Cristian Fogel, QA [:cfogel]]

Attachment: Screenshot 2020-11-12 at 08.28.06.png

The focus ring covers the entry-header bottom pixels, is that intended?
This is the case for 84.0a1 (2020-11-11) on the mentioned platforms.

Comment 5

[Emily McMinn :emcminn]

It wasn't really intentional as much as just the result of pushing the element down as little as possible to keep the ring from obscuring the date. We could always check with UX to see if the overlap is okay; the trade-off of moving it further would be margins/whitespace between the message titles and the date bar that are quite a bit bigger than the UX spec.

Comment 6

[Aaron Benson]

I don't think the whole content area should have a focus ring, just the Learn more link.

Comment 7

[Cristian Fogel, QA [:cfogel]]

With the confirmation from Aaron;
@Emily, do we need to file a followup bug or can this be re-patched?

Comment 8

[Emily McMinn :emcminn]

I'll take a look today; if it's a small enough change I'll reopen the bug and we can re-patch it.

Comment 9

[Emily McMinn :emcminn]

After looking in to this, I think I'm going to file a followup. The content area focus ring comes from the message being a button, which was originally done I believe because we wanted the entire area to be clickable/click through to the learn more page. It can be changed, but it'll require modifying the render of the WNPanel messages and the associated tests, so it's a bit bigger than a re-patch. Thx!

Comment 10

[Cristian Fogel, QA [:cfogel]]

Marking this as verified based on the comments above(the main issue with text being overlapped with focus ring has been addressed).
Will shift the QE+ flag on the followup bug and track rework-status on it.

@Emily thank you for the input and help with the issue.