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

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect)

Description

[Daniel Holbert [:dholbert]]

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).

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: screencast showing that the area to the right of the link is part of the active link target

Comment 2

[Daniel Holbert [:dholbert]]

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.

Comment 3

[Daniel Holbert [:dholbert]]

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.)

Comment 4

[Scott [:thecount] Downe]

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

Comment 5

[Daniel Holbert [:dholbert]]

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

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.

Comment 6

[Scott [:thecount] Downe]

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.

Comment 7

[Daniel Holbert [:dholbert]]

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

Comment 8

[Scott [:thecount] Downe]

Attachment: Bug 1610992 - Links in privacy modal width

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/520c60d7df26
Links in privacy modal width r=gvn

Comment 10

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/520c60d7df26

Comment 11

[Ryan VanderMeulen [:RyanVM]]

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

Comment 12

[Scott [:thecount] Downe]

[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.

Comment 13

[Daniel Holbert [:dholbert]]

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

Comment 14

[Scott [:thecount] Downe]

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

Comment 15

[Alin Ilea, Desktop QA]

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 16

[Pascal Chevrel:pascalc (PTO until April 26)]

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.

Comment 17

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/releases/mozilla-beta/rev/19f50e81ca55

Comment 18

[Alin Ilea, Desktop QA]

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