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

Status: RESOLVED FIXED

(DevTools :: Framework, defect)

Description

[Girish Sharma [:Optimizer]]

For example, if the tool does not support remote target, disable the checkbox and have a tooltip saying that target is not supported

Comment 1

[Girish Sharma [:Optimizer]]

Will disable the checkbox with a tooltip saying that target not supported.

Comment 2

[Girish Sharma [:Optimizer]]

Attachment: patch v1

Just calling isTargetSupported of the tool to see if target is supported. Adding a tooltip if not.

Comment 3

[Joe Walker [: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?

Comment 4

[Girish Sharma [:Optimizer]]

(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.

Comment 5

[Joe Walker [:jwalker]]

(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.

Comment 6

[Girish Sharma [:Optimizer]]

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.

Comment 7

[Joe Walker [:jwalker]]

(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?

Comment 8

[Girish Sharma [:Optimizer]]

Attachment: 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.

Comment 9

[Girish Sharma [:Optimizer]]

green try : https://tbpl.mozilla.org/?tree=Try&rev=7aca3fb3e46d

Comment 10

[Joe Walker [:jwalker]]

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!

Comment 11

[Girish Sharma [:Optimizer]]

(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.

Comment 12

[Girish Sharma [:Optimizer]]

Attachment: patch v2.1

Using * instead of the dagger as star is free now (see bug 881807)

Comment 13

[Girish Sharma [:Optimizer]]

Note that this bug and bug 881807 should be landed in the same push, this first then 881807

Comment 14

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/integration/fx-team/rev/694bcbab0f2f

Comment 15

[Victor Porof [:vporof][:vp]]

https://hg.mozilla.org/mozilla-central/rev/694bcbab0f2f