Closed Bug 736465 Opened 12 years ago Closed 12 years ago

Pref for Inspect Element context menu item

Categories

(DevTools :: Inspector, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 18

People

(Reporter: dangoor, Unassigned)

References

Details

Attachments

(1 file, 2 obsolete files)

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.
"devtools.inspector.hidecontextmenu" ?
Blocks: 736472
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.)
(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.
(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.
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.
(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
Honza created an issue for this:

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

Sebastian
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
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WONTFIX
actually, reopening. Do we want to do anything regarding a pref to explicitly disable the context menu?
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
We want devtools.inspector.devtools -> false to just disable the menuitem
hunh? Why that? why not devtools.inspector.contextMenu?
(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.
(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?
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.
(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!
Attached patch v1 (obsolete) — Splinter Review
Attachment #645925 - Flags: review?(dcamp)
Attachment #645925 - Flags: review?(dao)
Attachment #645925 - Flags: review?(dcamp) → review+
Dao: review ping
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
Attachment #645925 - Flags: review?(dao) → review-
OS: Mac OS X → All
Hardware: x86 → All
Attached patch v1.1 (obsolete) — Splinter Review
addressed Dao's comments
Attachment #645925 - Attachment is obsolete: true
Comment on attachment 656805 [details] [diff] [review]
v1.1

The bug that introduces the broadcaster broke this.
Attachment #656805 - Attachment is obsolete: true
Attached patch v1.2Splinter Review
Attachment #656811 - Flags: review?(dao)
Attachment #656811 - Flags: review?(dao) → review+
Whiteboard: [land-in-fx-team]
https://hg.mozilla.org/integration/fx-team/rev/01fba3ea3d13
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/01fba3ea3d13
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 18
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.