1325553 - Sound of "take screenshot" button should respect relevant preference

Status: VERIFIED FIXED

(DevTools :: Responsive Design Mode, defect)

Description

[el8]

User Agent: Mozilla/5.0 (Windows NT 6.0; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID: 20161208153507

Steps to reproduce:

Currently, if you click the camera icon on the Developer Tools toolbar to take a screenshot of the entire page, Firefox produces a camera clicking like sound; there's no way do disable it.
At the same time, if you switch to Responsive Design Mode and click the camera icon there to take a screenshot, no sound is produced (and there's no way to enable a sound for it).

It would be handy to be able enable/disable the sound for both icons with one preference.

Comment 1

[Tom Tromey :tromey]

> Firefox produces a camera clicking like sound; there's no way do disable it.

FWIW there is a pref for this and I see a checkbox for it in the devtools preferences panel.
This was added in bug 1257913.

What I've observed is that the RDM screenshot also makes the sound, but that the pref
doesn't seem to disable it.  That's because the RDM has its own implementation of
simulateCameraEffects that doesn't query the pref:

https://dxr.mozilla.org/mozilla-central/rev/2785aaf276ba29fb2e1f5607d90d441fee42efb4/devtools/client/responsive.html/actions/screenshot.js#57

Comment 2

[YF (Yang)]

https://dxr.mozilla.org/mozilla-central/rev/2785aaf276ba29fb2e1f5607d90d441fee42efb4/devtools/shared/gcli/commands/screenshot.js#190

simulateCameraEffect(context.environment.chromeDocument, "shutter");

Comment 3

[[:Towkir] Ahmed]

I could take this one if anyone is interested in mentoring.

Comment 4

[YF (Yang)]

I'm not a mentor.

Comment 6

[Tim Nguyen :ntim]

Missed comment 1

Comment 7

[Tim Nguyen :ntim]

(In reply to [:Towkir] Ahmed from comment #3)
> I could take this one if anyone is interested in mentoring.

On top of the file [0], you'll need to add `const Services = require("Services");`: this is necessary because the function you need to check the preference is on Services.

Then you'll need to wrap these 2 lines [1] with an if condition: `if (Services.prefs.getBoolPref("devtools.screenshot.audio.enabled")) {`

This makes sure the audio is only emitted when the pref is enabled. If you're interested in how the pref system works, see [2].

Towkir, let me know if you need extra info :)


[0] https://dxr.mozilla.org/mozilla-central/rev/2785aaf276ba29fb2e1f5607d90d441fee42efb4/devtools/client/responsive.html/actions/screenshot.js#57

[1] https://dxr.mozilla.org/mozilla-central/rev/2785aaf276ba29fb2e1f5607d90d441fee42efb4/devtools/client/responsive.html/actions/screenshot.js#57-58

[2] https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIPrefBranch#getBoolPref()

Comment 8

[[:Towkir] Ahmed]

Attachment: screenshotsound.patch

Hi Tim,
Hope this helps

Comment 9

[Tim Nguyen :ntim]

Comment on attachment 8822201 [details] [diff] [review]
screenshotsound.patch

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

Works great, thanks!

::: devtools/client/responsive.html/actions/screenshot.js
@@ +17,4 @@
>  const e10s = require("../utils/e10s");
>  
>  const CAMERA_AUDIO_URL = "resource://devtools/client/themes/audio/shutter.wav";
> +const Services = require("Services");

Can you move this require just after const e10s = require("...")

Comment 10

[[:Towkir] Ahmed]

Attachment: screenshotsound.patch

Missed the suggestion first time. Sorry about that.

Comment 11

[[:Towkir] Ahmed]

Hope it's alright to check-in now

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/47ead489b52e
Sound of "take screenshot" button respects relevant preference now; r=ntim

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/47ead489b52e

Comment 14

[Md. Majedul islam]

This bug  was about  to " add a preference to enable/disable the sound when click camera icon on the Developer Tools toolbar to take a screenshot "  I have seen the feature being implemented with latest Nightly  on Windows 10 , 64 Bit !

This bug's fix is now verified in Latest Nightly .

Build ID   :  20170103030204
User Agent : Mozilla/5.0 (Windows NT 10.0; WOW64; rv:53.0) Gecko/20100101 Firefox/53.0

[bugday-20170104]

Comment 15

[Wisdomjobs.com]

yes its working  https://www.wisdomjobs.com
thanks for your support

Comment 16

[shivi]

really its working.

<a href ="https://www.vacancyopen.com/"/>Thanks for posting.</a>

Comment 17

[shivi]

Its working https://www.vacancyopen.com/

Thanks for your support