Visually indicate that the tool is not supported on the target type in the Options panel

RESOLVED FIXED in Firefox 24

Status

()

Firefox
Developer Tools: Framework
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Optimizer, Assigned: Optimizer)

Tracking

unspecified
Firefox 24
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fixed-in-fx-team])

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
For example, if the tool does not support remote target, disable the checkbox and have a tooltip saying that target is not supported
(Assignee)

Comment 1

4 years ago
Will disable the checkbox with a tooltip saying that target not supported.
Assignee: nobody → scrapmachines
Status: NEW → ASSIGNED
(Assignee)

Comment 2

4 years ago
Created attachment 754959 [details] [diff] [review]
patch v1

Just calling isTargetSupported of the tool to see if target is supported. Adding a tooltip if not.
Attachment #754959 - Flags: review?(jwalker)
Comment on attachment 754959 [details] [diff] [review]
patch v1

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

This could be confusing couldn't it? By default we have disabled the new (theoretical) 'appcache' panel. Appcache is disabled whenever the site isn't using appcache.

Since it's disabled by default, people don't know about the appcache panel. You're browsing the web and you read about the new appcache panel, and there are instructions on how to enable it, so you then have to hunt around for a site that supports appcache in order to enable it.

Or did I misunderstand?
Attachment #754959 - Flags: review?(jwalker)
(Assignee)

Comment 4

4 years ago
(In reply to Joe Walker [:jwalker] from comment #3)
> Comment on attachment 754959 [details] [diff] [review]
> patch v1
> 
> Review of attachment 754959 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This could be confusing couldn't it? By default we have disabled the new
> (theoretical) 'appcache' panel. Appcache is disabled whenever the site isn't
> using appcache.
> 
> Since it's disabled by default, people don't know about the appcache panel.
> You're browsing the web and you read about the new appcache panel, and there
> are instructions on how to enable it, so you then have to hunt around for a
> site that supports appcache in order to enable it.

umm. I have no idea what you are talking about.

> Or did I misunderstand?

This bug is for the following purpose :

Open a remote toolbox. Inspector is still not remote-able, so it will not be present in the toolbox. But when you go to options panel, the Inspector tool is still checked. The user is not confused, he tries to uncheck it and then check it again, but Inspector does not appear.

Same goes for any add-on that does not support either remote target or window target.
(In reply to Girish Sharma [:Optimizer] from comment #4)
> (In reply to Joe Walker [:jwalker] from comment #3)
> > Comment on attachment 754959 [details] [diff] [review]
> > patch v1
> > 
> > Review of attachment 754959 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > This could be confusing couldn't it? By default we have disabled the new
> > (theoretical) 'appcache' panel. Appcache is disabled whenever the site isn't
> > using appcache.
> > 
> > Since it's disabled by default, people don't know about the appcache panel.
> > You're browsing the web and you read about the new appcache panel, and there
> > are instructions on how to enable it, so you then have to hunt around for a
> > site that supports appcache in order to enable it.
> 
> umm. I have no idea what you are talking about.
> 
> > Or did I misunderstand?
> 
> This bug is for the following purpose :
> 
> Open a remote toolbox. Inspector is still not remote-able, so it will not be
> present in the toolbox. But when you go to options panel, the Inspector tool
> is still checked. The user is not confused, he tries to uncheck it and then
> check it again, but Inspector does not appear.
> 
> Same goes for any add-on that does not support either remote target or
> window target.

So my understanding is correct.

Look at it another way - the options panel changes global options, but you're mixing target based information in with those global options.

We could perhaps indicate to the user with some text why the inspector doesn't appear ("Disabled for {reason}") or something. But altering the availability of global options based on locality doesn't make sense to me.
(Assignee)

Comment 6

4 years ago
I see now what you are trying to say. So instead of disabling, should I just mark it with different color somehow ? Adding a text would take a lot of space, thus I reverted to using tooltips. How do you suggest to show a text alongside the checkbox.
(In reply to Girish Sharma [:Optimizer] from comment #6)
> I see now what you are trying to say. So instead of disabling, should I just
> mark it with different color somehow ? Adding a text would take a lot of
> space, thus I reverted to using tooltips. How do you suggest to show a text
> alongside the checkbox.

There's lots of space just to the right of the tool name isn't there?
(Assignee)

Comment 8

4 years ago
Created attachment 759941 [details] [diff] [review]
patch v2

Using similar approach as in bug 864249, added a double dagger mark after the non supported tool, and just below list of all tools, adding the explanation.
Attachment #754959 - Attachment is obsolete: true
Attachment #759941 - Flags: review?(jwalker)
(Assignee)

Comment 9

4 years ago
green try : https://tbpl.mozilla.org/?tree=Try&rev=7aca3fb3e46d
Comment on attachment 759941 [details] [diff] [review]
patch v2

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

Given bug 881807, we could use * rather than the double dagger thing!
Attachment #759941 - Flags: review?(jwalker) → review+
(Assignee)

Comment 11

4 years ago
(In reply to Joe Walker [:jwalker] from comment #10)
> Given bug 881807, we could use * rather than the double dagger thing!

Agreed. Let me see if I can fix that quickly before this one to not have a key change.
(Assignee)

Updated

4 years ago
Blocks: 881807
(Assignee)

Comment 12

4 years ago
Created attachment 765560 [details] [diff] [review]
patch v2.1

Using * instead of the dagger as star is free now (see bug 881807)
Attachment #759941 - Attachment is obsolete: true
Attachment #765560 - Flags: review+
(Assignee)

Comment 13

4 years ago
Note that this bug and bug 881807 should be landed in the same push, this first then 881807
(Assignee)

Updated

4 years ago
Whiteboard: [land-in-fx-team]
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/694bcbab0f2f
Keywords: checkin-needed
Whiteboard: [land-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/694bcbab0f2f
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 24
You need to log in before you can comment on or make changes to this bug.