Last Comment Bug 736465 - Pref for Inspect Element context menu item
: Pref for Inspect Element context menu item
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Developer Tools: Inspector (show other bugs)
: unspecified
: All All
: -- normal with 2 votes (vote)
: Firefox 18
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
: 736472 (view as bug list)
Depends on:
Blocks: 736472
  Show dependency treegraph
 
Reported: 2012-03-16 07:48 PDT by Kevin Dangoor
Modified: 2012-09-05 04:21 PDT (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (1.23 KB, patch)
2012-07-25 16:15 PDT, Paul Rouget [:paul]
dcamp: review+
dao+bmo: review-
Details | Diff | Splinter Review
v1.1 (2.43 KB, patch)
2012-08-30 03:43 PDT, Paul Rouget [:paul]
no flags Details | Diff | Splinter Review
v1.2 (4.19 KB, patch)
2012-08-30 04:14 PDT, Paul Rouget [:paul]
dao+bmo: review+
Details | Diff | Splinter Review

Description Kevin Dangoor 2012-03-16 07:48:43 PDT
Advice has been circulating online in tweets and blog posts that people can turn off the built-in "Inspect Element" context menu item by setting "devtools.inspector.enabled" to false. These people want to turn off the context menu item because they will sometimes (often) get Page Inspector when they're looking to get Firebug's tool.

devtools.inspector.enabled is a blunt instrument for this job. As we roll out more new features for the Page Inspector (and other element-centric tools), people who have flipped this bit probably aren't as likely to try out those features because the page inspector won't even be in the Web Developer menu any more.

A pref for *just the context menu item* will give people finer control over this.
Comment 1 Paul Rouget [:paul] 2012-03-16 07:56:35 PDT
"devtools.inspector.hidecontextmenu" ?
Comment 2 Chris Cunningham 2012-06-04 02:09:19 PDT
What's the use case for "wanting Inspector enabled but not wanting the UI for it", exactly? I don't understand why there's a need to split the pref if nobody falls into that use case.

(For that matter, I can't understand why UI that literally nobody uses is enabled in production releases anyway, but there's not much point in raising a *third* bug which says "make sure this pref defaults to off" when the basic rationale for a UI-only pref seems unsound in the first place.)
Comment 3 Paul Rouget [:paul] 2012-06-04 02:15:11 PDT
(In reply to Chris Cunningham from comment #2)
> What's the use case for "wanting Inspector enabled but not wanting the UI
> for it", exactly? I don't understand why there's a need to split the pref if
> nobody falls into that use case.

You assume people don't want the builtin Inspector
What they don't want is the 2 menuitems next to eachother.

> (For that matter, I can't understand why UI that literally nobody uses is
> enabled in production releases anyway

People use the inspector today.
Comment 4 Jaymz Reiswig 2012-06-04 07:20:13 PDT
(In reply to Paul Rouget [:paul] from comment #3)
> (In reply to Chris Cunningham from comment #2)
> > What's the use case for "wanting Inspector enabled but not wanting the UI
> > for it", exactly? I don't understand why there's a need to split the pref if
> > nobody falls into that use case.
> 
> You assume people don't want the builtin Inspector
> What they don't want is the 2 menuitems next to eachother.

(As one of the "people" you're talking about...) I don't care about the built-in Inspector at all when I have Firebug installed. The rest of the dev tools are fine because they don't have a hook in my face, tripping me up every time I want to jump to a simple muscle-memory feature that has existed since I have been using Firebug.

FWIW, it probably makes sense for Firebug to add a pref that lets the user decide which inspector to show in the context menu. By default, Firebug should really remove Firefox's inspector from the context menu.
Comment 5 Rob Campbell [:rc] (:robcee) 2012-06-05 06:30:17 PDT
copied from bug 736472:

I would suggest:

devtools.inspector.contextMenu.enabled for your preference.

For users with explicitly-disabled devtools.inspector.enabled prefs, we could automatically set .contextMenu to false.

Jaymz' suggestion of passing the buck to Firebug to remove the built-in inspector context menu (or provide an option to remove it) is a pretty good one.

cc'ing Honza for consideration.
Comment 6 Jan Honza Odvarko [:Honza] PTO 07/23 - 08/08 2012-06-07 03:35:58 PDT
(In reply to Rob Campbell [:rc] (:robcee) from comment #5)
> Jaymz' suggestion of passing the buck to Firebug to remove the built-in
> inspector context menu (or provide an option to remove it) is a pretty good
> one.
> cc'ing Honza for consideration.
Thanks for heads up Rob. I put this on the list of things to discuss on the next Firebug weekly call.

Honza
Comment 7 Sebastian Zartner 2012-06-13 07:50:30 PDT
Honza created an issue for this:

http://code.google.com/p/fbug/issues/detail?id=5551

Sebastian
Comment 8 Sebastian Zartner 2012-06-28 00:02:08 PDT
The enhancement mentioned in comment 7 was integrated into Firebug 1.10b1 as extensions.firebug.hideDefaultInspector.

So I assume this issue can be closed.

Sebastian
Comment 9 Rob Campbell [:rc] (:robcee) 2012-07-19 12:27:01 PDT
*** Bug 774613 has been marked as a duplicate of this bug. ***
Comment 10 Rob Campbell [:rc] (:robcee) 2012-07-19 12:27:33 PDT
actually, reopening. Do we want to do anything regarding a pref to explicitly disable the context menu?
Comment 11 Paul Rouget [:paul] 2012-07-19 12:31:08 PDT
We want devtools.inspector.devtools -> false to just disable the menuitem
Comment 12 Rob Campbell [:rc] (:robcee) 2012-07-19 12:33:01 PDT
hunh? Why that? why not devtools.inspector.contextMenu?
Comment 13 Paul Rouget [:paul] 2012-07-19 12:38:06 PDT
(In reply to Rob Campbell [:rc] (:robcee) from comment #12)
> hunh? Why that? why not devtools.inspector.contextMenu?

Because people use the killswitch to disable the menuitem. So a lot of people have the whole inspector disabled because of that.
Comment 14 Rob Campbell [:rc] (:robcee) 2012-07-20 08:39:43 PDT
(In reply to Paul Rouget [:paul] from comment #13)
> (In reply to Rob Campbell [:rc] (:robcee) from comment #12)
> > hunh? Why that? why not devtools.inspector.contextMenu?
> 
> Because people use the killswitch to disable the menuitem. So a lot of
> people have the whole inspector disabled because of that.

Sorry, but I am confused by this now.

Initially, we had: devtools.inspector.enabled to killswitch the feature. That's gone now. People want a pref to disable the context menu (and, possibly the feature, but that's no longer a thing we want to do).

You're proposing we use: devtools.inspector.devtools to control the enablement of the context menu instead of devtools.inspector.contextMenu or the Developer Tools menu?
Comment 15 Paul Rouget [:paul] 2012-07-20 08:57:59 PDT
There were blog posts all over the internet about how to remove the "inspect" menuitem in the context menu, because people didn't like to have the 2 "inspect" menuitems. So I assume that a lot of people switched this pref off already. But we don't want these people to have the inspector disabled.

I'm saying here that we should kill the killswitch (no way to disable the inspector) and use the original pref as a way to remove the menuitem.

Alternatives: kill the killswitch. Use devtools.inspector.contextMenu, and initialize its value with the value of devtools.inspector.enabled.
Comment 16 Rob Campbell [:rc] (:robcee) 2012-07-25 11:38:27 PDT
(In reply to Paul Rouget [:paul] from comment #15)
> There were blog posts all over the internet about how to remove the
> "inspect" menuitem in the context menu, because people didn't like to have
> the 2 "inspect" menuitems. So I assume that a lot of people switched this
> pref off already. But we don't want these people to have the inspector
> disabled.

yes, ok.

> I'm saying here that we should kill the killswitch (no way to disable the
> inspector) and use the original pref as a way to remove the menuitem.

Right. That'd be ok.

So we'd use devtools.inspector.enabled to disable the context menu and not the feature. Got it!
 
> Alternatives: kill the killswitch. Use devtools.inspector.contextMenu, and
> initialize its value with the value of devtools.inspector.enabled.

Yeah, not necessary, I think. Just use the killswitch pref. Thanks!
Comment 17 Paul Rouget [:paul] 2012-07-25 16:15:08 PDT
Created attachment 645925 [details] [diff] [review]
v1
Comment 18 Paul Rouget [:paul] 2012-07-25 16:18:42 PDT
*** Bug 736472 has been marked as a duplicate of this bug. ***
Comment 19 Paul Rouget [:paul] 2012-08-15 03:20:07 PDT
Dao: review ping
Comment 20 Dão Gottwald [:dao] 2012-08-23 01:54:57 PDT
Comment on attachment 645925 [details] [diff] [review]
v1

I think you also need to get rid of INSP_ENABLED.
http://mxr.mozilla.org/mozilla-central/search?string=INSP_ENABLED
Comment 21 Paul Rouget [:paul] 2012-08-30 03:43:17 PDT
Created attachment 656805 [details] [diff] [review]
v1.1

addressed Dao's comments
Comment 22 Paul Rouget [:paul] 2012-08-30 03:47:23 PDT
Comment on attachment 656805 [details] [diff] [review]
v1.1

The bug that introduces the broadcaster broke this.
Comment 23 Paul Rouget [:paul] 2012-08-30 04:14:29 PDT
Created attachment 656811 [details] [diff] [review]
v1.2
Comment 25 Tim Taubert [:ttaubert] 2012-09-05 04:21:36 PDT
https://hg.mozilla.org/mozilla-central/rev/01fba3ea3d13

Note You need to log in before you can comment on or make changes to this bug.