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)
Tracking
()
People
(Reporter: dholbert, Assigned: thecount)
Details
Attachments
(3 files)
253.25 KB,
video/ogg
|
Details | |
291.83 KB,
image/png
|
Details | |
47 bytes,
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details | Review |
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.
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).
Reporter | ||
Comment 1•5 years ago
|
||
Reporter | ||
Comment 2•5 years ago
|
||
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.
Reporter | ||
Comment 3•5 years ago
|
||
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.)
Reporter | ||
Updated•5 years ago
|
Assignee | ||
Comment 4•5 years ago
|
||
This looks like an easy flex to inline-flex on the link.
Reporter | ||
Comment 5•5 years ago
•
|
||
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.
Reporter | ||
Updated•5 years ago
|
Assignee | ||
Comment 6•5 years ago
|
||
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.
Reporter | ||
Comment 7•5 years ago
|
||
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 | ||
Comment 8•5 years ago
|
||
Updated•5 years ago
|
Comment 10•5 years ago
|
||
bugherder |
Comment 11•5 years ago
|
||
Is this something we should consider uplifting to Beta for 74 or can it ride 75 to release?
Assignee | ||
Comment 12•5 years ago
|
||
[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.
Reporter | ||
Comment 13•5 years ago
|
||
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
Updated•5 years ago
|
Assignee | ||
Comment 14•5 years ago
|
||
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
Assignee | ||
Updated•5 years ago
|
Updated•5 years ago
|
Comment 15•5 years ago
|
||
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•5 years ago
|
||
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•5 years ago
|
||
bugherder uplift |
Comment 18•5 years ago
|
||
Verified - Fixed in latest Beta 74.0b5 (Build id: 20200218224219) using Windows 10, Mac OS 10.13 and Ubuntu 18.04.
Description
•