tooltip description is wrong for the camera icon.

VERIFIED FIXED in Firefox 43

Status

Hello (Loop)
Client
P4
normal
Rank:
45
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: situmam, Assigned: JoelBrewer01)

Tracking

unspecified
mozilla43
Points:
1
Bug Flags:
firefox-backlog +

Firefox Tracking Flags

(firefox43 verified)

Details

(Whiteboard: [good first bug][lang=js])

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

3 years ago
Created attachment 8641048 [details]
mute_video.png

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:41.0) Gecko/20100101 Firefox/41.0
Build ID: 20150727205026

Steps to reproduce:

* Launch latest FF nightly
* Click on Hello button
* hover mouse over the camera icon


Actual results:

When hovering over the camera logo, a tooltip (bubble) shows up saying "mute your video".


Expected results:

This is incorrect. It should be called "turn off camera"
(Reporter)

Updated

3 years ago
Summary: mute your video → tooltip description is wrong for the camera icon.
Sevaan: any suggestions for wording improvements here?

"Turn off camera" isn't quite right as we don't currently turn off the camera - we just stop sending the video (the camera turn-off support would require extra work where we haven't got full platform support yet).
Flags: needinfo?(sfranks)
How about "Disable camera"?
Flags: needinfo?(sfranks)
(Reporter)

Comment 3

3 years ago
Sounds like "Disable video" is more appropriate since you don't turn the camera off.
"Disabled video" is great.

Updated

3 years ago
Rank: 45
Flags: qe-verify+
Priority: -- → P4
Whiteboard: [good first bug]
(Assignee)

Comment 5

3 years ago
Hello! This is my first foray into Mozilla open source development and this looks like an easy bug for me to start out with. Looks like it hasn't been claimed yet?

Updated

3 years ago
Assignee: nobody → JoelBrewer01
(In reply to JoelBrewer01 from comment #5)
> Hello! This is my first foray into Mozilla open source development and this
> looks like an easy bug for me to start out with. Looks like it hasn't been
> claimed yet?

Hi, its now assigned to you, so you've got it :-)

As its your first time, I'll offer a few hints straight up:

- There's more than one file in which you need to change the string.
- Additionally, you need to change the string id for our localisation folks to see the change. I think you can just add a 2 on the end of the string id.
- You'll need to update the code as well because of the string id change, in this case, our code is in the /browser/components/loop directory, and you need to look for "button_title" - you won't find the string id as a whole.

You probably will also want to check the unit tests work.

There's more information on Hello (aka Loop) development here as well:

https://wiki.mozilla.org/Loop/Development
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [good first bug] → [good first bug][lang=js]
(Assignee)

Comment 7

3 years ago
Created attachment 8648434 [details] [diff] [review]
bug1189287patch1 - Here is my first attempt at a fix for this bug.

My first patch to Mozilla! :)

I updated the string as well as the string id. I also updated what I believe is the relevant code in the views.js file.
Attachment #8648434 - Flags: review?(mdeboer)
(Assignee)

Comment 8

3 years ago
Ah. I found a bug in that review. About to upload an updated patch.
(Assignee)

Comment 9

3 years ago
Created attachment 8648435 [details] [diff] [review]
bug1189287patch2 - Updated patch #2.

Here is my updated patch. Realized I wasn't accounting for the different string ID for the audio button.
Attachment #8648434 - Attachment is obsolete: true
Attachment #8648434 - Flags: review?(mdeboer)
Attachment #8648435 - Flags: ui-review?(mdeboer)
(Assignee)

Updated

3 years ago
Attachment #8648435 - Flags: ui-review?(mdeboer) → review?(mdeboer)
Iteration: --- → 43.1 - Aug 24
Points: --- → 1
Flags: firefox-backlog+
Comment on attachment 8648435 [details] [diff] [review]
bug1189287patch2 - Updated patch #2.

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

Joel, that's an awesome first patch - congratulations!!

There's one thing you need to change: add the new strings to the file 'browser/locales/en-US/chrome/browser/loop/loop.properties' as well, so that you can see the change in the desktop client as well.

Can you tell me how you tested the change you made? If you have any questions, please don't hesitate to ask on IRC in #loop or me directly: mikedeboer.

I'm looking forward to the next version of your patch!
Attachment #8648435 - Flags: review?(mdeboer) → feedback+
Status: NEW → ASSIGNED
(Assignee)

Comment 11

3 years ago
Created attachment 8650266 [details] [diff] [review]
bug1189287patch3 - Updated patch #3

Alright, here's the updated patch. I updated the second language file. I also used '===' instead of '===' in my js code.

For testing, I ran: '/browser/components/loop/run-all-loop-tests.sh' and as far as I can tell they all passed :)
Attachment #8648435 - Attachment is obsolete: true
Attachment #8650266 - Flags: review?(mdeboer)
Comment on attachment 8650266 [details] [diff] [review]
bug1189287patch3 - Updated patch #3

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

I can't see anything wrong with this patch, congratulations!!

When this lands on our code repository, I'll be filing a follow-up to make the `_getTitle` function handle new string IDs better. But for now, we're good to go!

Thanks for working on this and I'm looking forward to seeing your next patch ;-)
Attachment #8650266 - Flags: review?(mdeboer) → review+
url:        https://hg.mozilla.org/integration/fx-team/rev/dfc8a6f5a9e2e8854317e442828d01f80add633b
changeset:  dfc8a6f5a9e2e8854317e442828d01f80add633b
user:       jahbrewski <joelbrewer01@gmail.com>
date:       Thu Aug 20 11:33:28 2015 +0200
description:
Bug 1189287 - Update the tooltip strings for the mute and unmute video buttons inside the Hello conversation toolbar. r=mikedeboer
https://hg.mozilla.org/mozilla-central/rev/dfc8a6f5a9e2
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
QA Contact: bogdan.maris
Verified that the tooltip description has changed to 'Disable video' using latest Nightly 43.0a1 across platforms (Windows 7 64-bit, Mac OS X 10.10.5 and Ubuntu 14.04 32-bit).
Status: RESOLVED → VERIFIED
status-firefox43: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.