Add tooltip to say what "ev" means

VERIFIED FIXED in Firefox 57

Status

()

Firefox
Developer Tools: Inspector
P3
normal
VERIFIED FIXED
a year ago
8 months ago

People

(Reporter: Dan Jacobson, Assigned: Aastha, Mentored)

Tracking

({good-first-bug})

53 Branch
Firefox 57
good-first-bug
Points:
---

Firefox Tracking Flags

(firefox57 verified)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

a year ago
One sees "Button: [ev]" but there is not tooltip to pop up and say what "ev" means or might do.
(Reporter)

Comment 1

a year ago
Same with Bubbing and Domo.
(Reporter)

Comment 2

a year ago
Their current tooltips are just their names. That can never be right.
Good point, the (ev) icon should definitely have a title tooltip so people know what it is about.
And the DOM0/1/2 and Bubbling tags just have silly tooltips.

This is an easy bug if someone wants to work on it. I'm happy to help.
Make sure you go over our contribution guide first: https://developer.mozilla.org/en-US/docs/Tools/Contributing

Once ready, you might be interested in looking at the following code locations:

/devtools/client/inspector/markup/markup.xhtml, in this file, the div.markupview-events element is the little (ev) icon in the inspector. Adding a title attribute to it would work fine.

In devtools/client/shared/widgets/tooltip/EventTooltipHelper.js we create the content of the event tooltips, including the little DOM0/1/2 and Bubbling tags. Here's the exact code position where the title attribute is added:
http://searchfox.org/mozilla-central/rev/b1aadb3572eaf7d2c70e19a2ba5413809d9ac698/devtools/client/shared/widgets/tooltip/EventTooltipHelper.js#137
As you can see, the title is just the same text as the tag itself. We should introduce an actual localized string for this instead.

devtools/client/locales/en-US/inspector.properties is the properties file that contains our inspector localized strings. So it would be the right place to add the new titles.

Let me know if you need more help.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: good-first-bug
Priority: -- → P3
Mentor: pbrosset

Comment 4

a year ago
Hi Patrick, 
Thanks for mentoring, I will submit a patch soon, assigning for now, and will let you know if I need anything.
Thanks
Assignee: nobody → 3ugzilla
Status: NEW → ASSIGNED
(Assignee)

Comment 5

8 months ago
Created attachment 8905288 [details] [diff] [review]
added tooltip text for ev button
(Assignee)

Comment 6

8 months ago
Hi,
This is my first patch in Mozilla community.Kindly review it and let me know if any changes are required.
Thanks
Thank you very much for getting involved on the project! Your code changes look pretty good at first glance.
I'm going to pass mentorship of this bug to Mike who knows this part better than I do and will be able to help you and review your code changes.
It would also be good to update the DOM0/1/2/ and Bubbling tooltips, I don't see this being changed in your patch.
Anyway, great start. I'll assign the bug to you now.
Assignee: 3ugzilla → aastha.gupta4104
Mentor: pbrosset → mratcliffe

Updated

8 months ago
Attachment #8905288 - Flags: review?(mratcliffe)
(In reply to Aastha from comment #6)
> Hi,
> This is my first patch in Mozilla community.Kindly review it and let me know
> if any changes are required.
> Thanks

Nice work, thanks for contributing.

Do you think you could also remove the DOM0/1/2/ and Bubbling tooltips then re-upload the patch?
Flags: needinfo?(aastha.gupta4104)
(Assignee)

Comment 9

8 months ago
Created attachment 8905506 [details] [diff] [review]
added inspector localised strings for DOM0, DOM2, Bubbling
Attachment #8905288 - Attachment is obsolete: true
Attachment #8905288 - Flags: review?(mratcliffe)
Flags: needinfo?(aastha.gupta4104)
I didn't realize that you were localizing the tooltips. Just a couple of changes and you are done.

Can you create a single patch with the following changes?

1. Add a tooltip to the ev button.
2. Remove the tooltips from the DOM0 and DOM2 tags because they are the same in every language.
3. Localize the tooltips for Capturing and Bubbling (move them to the .properties file).
Flags: needinfo?(aastha.gupta4104)
(Assignee)

Comment 11

8 months ago
Created attachment 8906343 [details] [diff] [review]
Made all the changes that had to be made

1. Added a tooltip to the ev button.
2. Removed the tooltips from the DOM0 and DOM2 tags.
3. Localized the tooltips for Capturing and Bubbling.
Attachment #8905506 - Attachment is obsolete: true
Flags: needinfo?(aastha.gupta4104)
Comment on attachment 8906343 [details] [diff] [review]
Made all the changes that had to be made

Review of attachment 8906343 [details] [diff] [review]:
-----------------------------------------------------------------

Perfect, great job!
Attachment #8906343 - Flags: review+
Keywords: checkin-needed

Comment 13

8 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/336effc1fe44
Add tooltip to say what "ev" means in the Inspector. r=miker
Keywords: checkin-needed

Comment 14

8 months ago
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e2f9e7b7fd7
Add tooltip to say what "ev" means in the Inspector. Follow-up: Fix long line. r=eslint-fix

Comment 15

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/336effc1fe44
https://hg.mozilla.org/mozilla-central/rev/0e2f9e7b7fd7
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57

Comment 16

8 months ago
I have reproduced this Bug with nightly 54.0a1 (2017-02-02) on Ubuntu 16.04, 64 Bit!

The bug's fix is now verified on latest nightly 57.0a1

Build ID     20170912100139

User Agent   Mozilla/5.0 (X11; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0
QA Whiteboard: [bugday-20170913]

Comment 17

8 months ago
I have successfully reproduced this bug with Nightly 57.0a1 (2017-02-02) on windows 10 (32-bit)

this bug is verified fix with  latest nightly 57.0a1 (2017-09-12) (32-bit)

Build ID: 20170912100139
Mozilla/5.0 (Windows NT 10.0; rv:57.0) Gecko/20100101 Firefox/57.0

[bugday-20170913]

Comment 18

8 months ago
As per Comment 16 and Comment 17, I am marking this bug as verified fixed.
Status: RESOLVED → VERIFIED
status-firefox57: fixed → verified
You need to log in before you can comment on or make changes to this bug.