Closed Bug 1619517 Opened 5 months ago Closed 3 months ago

Tooltip missing for the the "X" button inside the cards

Categories

(Firefox :: Protections UI, defect, P2)

75 Branch
defect

Tracking

()

RESOLVED FIXED
Firefox 77
Tracking Status
firefox75 --- wontfix
firefox76 --- wontfix
firefox77 --- fixed

People

(Reporter: simona.marcu, Assigned: shaynajustus, Mentored)

References

(Blocks 1 open bug)

Details

(Keywords: good-first-bug)

Attachments

(1 file, 1 obsolete file)

Affected versions

  • latest Nightly 75.0a1

Affected platforms

  • Windows 10 x64
  • macOS 10.15
  • Ubuntu 18.04 x64

Prerequisites

  • In about:config, set the preference "browser.contentblocking.report.show_mobile_app" to "true"

Steps to reproduce

  1. Launch Firefox and navigate to "about:protections".
  2. Scroll down the page and go to the "Mobile Call Card" section.
  3. Hover your mouse over the "X" button.

Expected result

  • The “X” button is focused and a proper tooltip is shown on hover.

Actual result

  • The “X” button is focused but no there is no tooltip.

Simona, what is the copy for this tooltip text? Where can I access our UX spec for this?

I'd like to mentor this bug.

Mentor: prathikshaprasadsuman
Flags: needinfo?(simona.marcu)

Eric can you post a tooltip for the X on the mobile promotion card?
Parthiksha, you can find the UX spec here, at the moment it is missing the tooltip.

Flags: needinfo?(simona.marcu) → needinfo?(epang)
Keywords: good-first-bug
Priority: -- → P2

Spoke with Meridel about this. We can use "Close" as the tooptip thanks!

Flags: needinfo?(epang)

Hi! I would like to be assigned this bug.

Assignee: nobody → shaynajustus
Status: NEW → ASSIGNED

I added an html title attribute set to "Close" on the for the x icon in the Mobile Call Card and now the tooltip appears, but I want to make sure I'm on the right track. I looked for a another tooltip on the page to see how a tooltip had been implemented previously and found that when you hover/focus over the "Privacy Details" section you get a tooltip that says "Go to Privacy Settings". When I used Inspect Element on the page I saw that it has a title attribute that gave the tooltip. But I can't find where the tooltip is coming from in the code. In protections.html the title attribute doesn't appear to be hard coded, and I don't see how it's implemented in the JS (if it is). That makes me wonder if there's a better way to implement the tooltip that I'm not seeing?

Flags: needinfo?(prathikshaprasadsuman)

Depends on D66975

(In reply to Shayna from comment #5)

I added an html title attribute set to "Close" on the for the x icon in the Mobile Call Card and now the tooltip appears, but I want to make sure I'm on the right track. I looked for a another tooltip on the page to see how a tooltip had been implemented previously and found that when you hover/focus over the "Privacy Details" section you get a tooltip that says "Go to Privacy Settings". When I used Inspect Element on the page I saw that it has a title attribute that gave the tooltip. But I can't find where the tooltip is coming from in the code. In protections.html the title attribute doesn't appear to be hard coded, and I don't see how it's implemented in the JS (if it is). That makes me wonder if there's a better way to implement the tooltip that I'm not seeing?

You're right. The tool tip strings are not hard coded in protections.html. If you search for "Go to Privacy Settings" in https://searchfox.org/, you'll see that it is added in this .ftl file[0]. Similarly, you can add your new tool tip string here[1] and that should automatically make the tool tip appear in "about:protections" because "data-l10n-id" is set as "protections-close-button" for the "X" button in protections.html[2].

[0] https://searchfox.org/mozilla-central/rev/c80fa7258c935223fe319c5345b58eae85d4c6ae/browser/locales/en-US/browser/protections.ftl#32
[1] https://searchfox.org/mozilla-central/rev/c80fa7258c935223fe319c5345b58eae85d4c6ae/browser/locales/en-US/browser/protections.ftl#67
[2] https://searchfox.org/mozilla-central/rev/c80fa7258c935223fe319c5345b58eae85d4c6ae/browser/components/protections/content/protections.html#119

Flags: needinfo?(prathikshaprasadsuman)

(In reply to :prathiksha from comment #7)

(In reply to Shayna from comment #5)

I added an html title attribute set to "Close" on the for the x icon in the Mobile Call Card and now the tooltip appears, but I want to make sure I'm on the right track. I looked for a another tooltip on the page to see how a tooltip had been implemented previously and found that when you hover/focus over the "Privacy Details" section you get a tooltip that says "Go to Privacy Settings". When I used Inspect Element on the page I saw that it has a title attribute that gave the tooltip. But I can't find where the tooltip is coming from in the code. In protections.html the title attribute doesn't appear to be hard coded, and I don't see how it's implemented in the JS (if it is). That makes me wonder if there's a better way to implement the tooltip that I'm not seeing?

You're right. The tool tip strings are not hard coded in protections.html. If you search for "Go to Privacy Settings" in https://searchfox.org/, you'll see that it is added in this .ftl file[0]. Similarly, you can add your new tool tip string here[1] and that should automatically make the tool tip appear in "about:protections" because "data-l10n-id" is set as "protections-close-button" for the "X" button in protections.html[2].

[0] https://searchfox.org/mozilla-central/rev/c80fa7258c935223fe319c5345b58eae85d4c6ae/browser/locales/en-US/browser/protections.ftl#32
[1] https://searchfox.org/mozilla-central/rev/c80fa7258c935223fe319c5345b58eae85d4c6ae/browser/locales/en-US/browser/protections.ftl#67
[2] https://searchfox.org/mozilla-central/rev/c80fa7258c935223fe319c5345b58eae85d4c6ae/browser/components/protections/content/protections.html#119

Wow that makes so much more sense now. Thanks! I realized I forgot to pull the most recent code base before I started and my code base was a little different. Instead of a button with the "data-l10n-id" attribute, I had a span with an aria-label. I was trying to figure out how those data attributes worked and it makes so much more sense now.

I used hg amend and resubmitted my code but I didn't see a confirmation with hg amend and I'm not sure how to be certain it worked.

(In reply to Shayna from comment #8)

I used hg amend and resubmitted my code but I didn't see a confirmation with hg amend and I'm not sure how to be certain it worked.

Hi Shayna, I'm not sure you pushed your updated patch for review? I don't see an update on Phabricator or any review requests from you in my queue. Do you need help submitting this patch? If you have an amended patch, you can still use moz-phab submit to push it.

Flags: needinfo?(shaynajustus)

I thought I had submitted it but I was confused because I nothing seemed to change on Phabricator. I just re-submitted and checked Phabricator. I think it's showing now. Hopefully it worked.

Flags: needinfo?(shaynajustus)
Flags: needinfo?(prathikshaprasadsuman)

Responded on Phabricator

Flags: needinfo?(prathikshaprasadsuman)
Attachment #9135004 - Attachment is obsolete: true

Hello, is this bug still active? And if no one is working on it right now, I'd like to look into it.

Shayna, are you still working on this?

Flags: needinfo?(shaynajustus)

Yes, I'm still working on it. Trying to implement the migration and I keep getting an error, but I haven't been able to work in it for the last several days. Going to to try to figure it out this weekend.

Flags: needinfo?(shaynajustus)

(In reply to Shayna from comment #16)

Yes, I'm still working on it. Trying to implement the migration and I keep getting an error, but I haven't been able to work in it for the last several days. Going to to try to figure it out this weekend.

What kind of error? If you post here, in matrix or on phabricator, people can help. :-)

(In reply to :Gijs (he/him) from comment #17)

(In reply to Shayna from comment #16)

Yes, I'm still working on it. Trying to implement the migration and I keep getting an error, but I haven't been able to work in it for the last several days. Going to to try to figure it out this weekend.

What kind of error? If you post here, in matrix or on phabricator, people can help. :-)

I think I figured out the first error I got, but now I'm getting this error:

0:01.14 ERROR in python/l10n/fluent_migrations/bug_1619517_close_tooltip_mobile_card.py: Transform contains parse error: Expected an entry start, at 1
0:01.14 hg pull -u

(In reply to Shayna from comment #18)

0:01.14 ERROR in python/l10n/fluent_migrations/bug_1619517_close_tooltip_mobile_card.py: Transform contains parse error: Expected an entry start, at 1
0:01.14 hg pull -u

Can you hg add the migration and push to phabricator, passing --wip to moz-phab , so we can see what code is triggering this?

Flags: needinfo?(shaynajustus)

I've added the migration and pushed my code.

Flags: needinfo?(shaynajustus)

(In reply to Shayna from comment #20)

I've added the migration and pushed my code.

Phabricator does not show a push.

Flags: needinfo?(shaynajustus)

(In reply to :Gijs (he/him) from comment #21)

(In reply to Shayna from comment #20)

I've added the migration and pushed my code.

Phabricator does not show a push.

Sorry about that -- I forgot to amend the commit before I submitted it. Should have double checked it before I went to work yesterday, but I think it worked this time.

Flags: needinfo?(shaynajustus)

(In reply to Shayna from comment #22)

Sorry about that -- I forgot to amend the commit before I submitted it. Should have double checked it before I went to work yesterday, but I think it worked this time.

Yep, it worked - I replied on phab with how you can make the migration recipe work - https://phabricator.services.mozilla.com/D69308#2215886 :-)

It looks like landing failed. Shayna, can you rebase this patch on central? There's instructions on how to do that in our hackMD document. I can also help you with this on Matrix.

Flags: needinfo?(shaynajustus)

I think the rebase worked and I re-submitted my code.

Flags: needinfo?(shaynajustus)
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/5c6cf6eaf47c
Add Close tooltip to Mobile Call Card in about:protections r=prathiksha,fluent-reviewers,Gijs,flod
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 77

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

QA Whiteboard: [qa-77b-p2]
You need to log in before you can comment on or make changes to this bug.