Closed Bug 1610992 Opened 1 year ago Closed 1 year ago

In new-tab sponsors/privacy modal popup, the "Learn how privacy works on the new tab" link has a wider-than-expected click area

Categories

(Firefox :: New Tab Page, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 75
Tracking Status
firefox74 --- verified
firefox75 --- verified

People

(Reporter: dholbert, Assigned: thecount)

Details

Attachments

(3 files)

STR:

  1. In a new tab, hover some sponsored tile and click its menu, and choose "Our sponsors and your privacy".
  2. Hover the area to the right of the two links in the modal popup.

ACTUAL RESULTS:

  • The blank space to the right of the "Learn how privacy works on the new tab" link is mysteriously linkified (that blank space is part of the link's hover-target).
  • ...vs. the blank space next to the other link is not linkified.
  • This inconsistency seems broken (and the linkified blank space feels especially broken).

EXPECTED RESULTS:

  • The click target for the "Learn how privacy works on the new tab" link should not extend beyond the text itself.

I'm tentatively marking this as confidential since this is part of the feature in bug 1602140 which is also marked as confidential (though the feature has shipped in Nightly, so I'm not sure this needs to be confidential).

The a element is wide here because it has display:flex which makes it block-level (rather than inline-level), which means its default width:auto value expands to fill its (wide) container.

In contrast, the lower link ("Manage sponsored content settings") is actually a <button> for some reason, and buttons have different behavior (due to being semi-replaced elements) which means they don't stretch to fill their container like other block-level content would.

So, that's why these two links behave differently. I see two easy ways to fix the discrepancy:
(1) Make the first element a button, too (for consistency) -- it's weird that these two similar-looking links are using different element types.
...or:
(2) Make the first element have width: max-content (instead of width:auto). This is the most targeted way to make it size to its content, without changing other things about the layout.

sdowne, looks like you worked on this feature - maybe you could take a look?

(Not sure about its ride-the-trains status given bug 1602140 comment 32 -- but it is shipping in Nightly at least, so it seems worth a look.)

Flags: needinfo?(sdowne)
Group: mozilla-employee-confidential

This looks like an easy flex to inline-flex on the link.

Flags: needinfo?(sdowne)

Thanks for taking a look! But no, it's not quite that simple. inline-flex would kind of work, but it's fragile, because it means the links (now both inline-level) could end up side-by-side if they're skinny enough to fit. We'd be depending on the precise text content being a particular minimum width, in order to produce the expected layout. (And in certain locales/languages/fonts, it may or may not satisfy that minimum width.)

Here's a screenshot showing that possible broken-layout -- I've simply used devtools to switch the first a to have to display:inline-flex and I've shortened the text of the first link, swapping in [SNIP] for the second half of the sentence. As you can see, the links then end up side-by-side instead of one above the other.

If you switch to inline-flex and add a br between the links, that'd avoid this potential issue, I think. Or you could leave its display value as it currently stands and just give it a width as noted previously here, or do any of several other things.

Attachment #9122913 - Attachment description: screenshot demonstrating that `inline-flex` is not a sufficient solution → screenshot demonstrating that `inline-flex` (on its own) is not a sufficient solution

Ah right, about inline-flex not be correct. That wouldn't work very well.

I like width: max-content the best, I'll look at that.

The reason one is a button and the other a link, is the button takes you to about:preferences#home which you cannot use in a simple href (I don't know the technical reasons as to why not at this time)

It is inconsistent, and I'm not sure about the trade off on UX of making the link a button, to consistency, but I don't think it's a huge issue and probably we can just use max-content anyway.

Sounds good to me! Yeah, I don't know that link/button makes a huge difference here, as long as it looks/behaves correctly.

Assignee: nobody → sdowne
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 75
See Also: → 1614787

Is this something we should consider uplifting to Beta for 74 or can it ride 75 to release?

Flags: needinfo?(sdowne)

[Tracking Requested - why for this release]:

It's low risk so probably worth it.

If it doesn't happen though, I think that's fine.

Flags: needinfo?(sdowne)

You requested tracking, but I think you really want to request approval-mozilla-beta on the patch (it's a flag available on the "details" page for the attachment here, which is https://bugzilla.mozilla.org/attachment.cgi?id=9122916&action=edit )

That'll give you some boilerplate questions to fill out, in the comment-box there. Once it's been granted approval, someone will go ahead and land it for you.

More info (mostly targeted at release managers) on https://wiki.mozilla.org/Release_Management/Uplift_rules

Flags: needinfo?(sdowne)

Comment on attachment 9122916 [details]
Bug 1610992 - Links in privacy modal width

Beta/Release Uplift Approval Request

  • User impact if declined: Some UX wonkyness

  • Is this code covered by automated tests?: Yes

  • Has the fix been verified in Nightly?: No

  • Needs manual test from QE?: Yes

  • If yes, steps to reproduce: STR:

    In a new tab, hover some sponsored tile and click its menu, and choose "Our sponsors and your privacy".
    Hover the area to the right of the two links in the modal popup.

ACTUAL RESULTS:

The blank space to the right of the "Learn how privacy works on the new tab" link is mysteriously linkified (that blank space is part of the link's hover-target).
...vs. the blank space next to the other link is not linkified.
This inconsistency seems broken (and the linkified blank space feels especially broken).

EXPECTED RESULTS:

The click target for the "Learn how privacy works on the new tab" link should not extend beyond the text itself.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): It's a very minimal amount of css
  • String changes made/needed: none
Flags: needinfo?(sdowne)
Attachment #9122916 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

Reproduced the initial issue in Beta 74.0b4 (Build id: 20200216164042) using Windows 10.
Verified - Fixed in latest Nightly 75.0a1 (Build id: 20200216210001) using Windows 10, Mac OS 10.13 and Ubuntu 18.04.

Comment on attachment 9122916 [details]
Bug 1610992 - Links in privacy modal width

Low risk CSS patch, verified on nightly by QA, uplift approved for 74.0b5, thanks.

Attachment #9122916 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Verified - Fixed in latest Beta 74.0b5 (Build id: 20200218224219) using Windows 10, Mac OS 10.13 and Ubuntu 18.04.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.