1361146 - [devtools-addon] Clarify UITour dependencies on devtools

Status: RESOLVED INVALID

(DevTools :: General, defect, P3)

Description

[Julian Descottes [:jdescottes]]

The UITour lib (browser/components/uitour/) can trigger devtools related highlights.

The tours are triggered from content scripts loaded on whitelisted URLs. For instance https://www.mozilla.org/en-US/firefox/54.0a2/firstrun/ will trigger a devtools specific UI tour.

The devtools specific targets are:
- developer-button
- webide

After devtools move out of mozilla-central, these buttons might no longer be available if devtools are not installed. This depends on what we decide for the UI shim of devtools (Bug 1361080). 

If we decide to keep the buttons, we only have to update the current test in order to guard reading the devtools webide pref [1] (otherwise the pref will not be available and getBoolPref will throw).

If we decide to get rid of the buttons, we need to:
- update the test at browser/components/uitour/test/browser_UITour_availableTargets.js
- update browser/components/uitour/UITour.jsm to remove developer-button and webide as valid targets

[1] http://searchfox.org/mozilla-central/rev/3dc6ceb42746ab40f1441e1e659ffb8f62ae78e3/browser/components/uitour/test/browser_UITour_availableTargets.js#7

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Assuming the WebIDE is button is not just removed entirely, we should probably at least remove the "mysterious" behavior it has.  Currently, the button is auto-installed into the navbar for Dev. Edition, and on other channels it's auto-moved there on first open of WebIDE, both of which have angered people over the years.

The following prefs[1] and code[2][3] are related to this behavior.  My opinion would be to keep the button around, but just let it be off by default, so you'd have to drag it out of the customize panel, but I also don't feel strongly since we want to remove WebIDE entirely anyway.  (There's still a menu item and shortcut as well.)

[1]: http://searchfox.org/mozilla-central/rev/ae8c2e2354db652950fe0ec16983360c21857f2a/devtools/client/webide/webide-prefs.js#25-32
[2]: http://searchfox.org/mozilla-central/rev/ae8c2e2354db652950fe0ec16983360c21857f2a/devtools/client/webide/content/webide.js#109-113
[3]: http://searchfox.org/mozilla-central/rev/ae8c2e2354db652950fe0ec16983360c21857f2a/devtools/client/framework/devtools-browser.js#117-122

Comment 2

[Julian Descottes [:jdescottes]]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #1)
> Assuming the WebIDE is button is not just removed entirely, we should
> probably at least remove the "mysterious" behavior it has.  Currently, the
> button is auto-installed into the navbar for Dev. Edition, and on other
> channels it's auto-moved there on first open of WebIDE, both of which have
> angered people over the years.

Good to know! We actually have Bug 1183962 which is precisely about the "auto-move" on first open. I was about to remove that, but if we say we also want to get rid of the button on dev-edition, I can do that at the same time. Will make the devtools-addon cleanup easier.

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: Bug 1361146 - remove developer button from the valid UI Tour targets

Review commit: https://reviewboard.mozilla.org/r/141794/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/141794/

Comment 4

[Julian Descottes [:jdescottes]]

Bryan, now that bug 1183962 is fixed, we only have the developer button as UITour target. 
This button will always be available, either for the shim, or for the actual devtools. 

We could leave it as is and remain a valid UITour target.
Or as I propose in Bug 1361081, use the Shim to get the button id, to clarify dependencies.

But I know we also mentioned removing ourselves from the available UITour targets. Do you feel like we should do that now? (I attached a patch that does that, in case).

Comment 5

[Bryan Clark (DevTools PM) [:clarkbw]]

(In reply to Julian Descottes [:jdescottes] from comment #4)
> We could leave it as is and remain a valid UITour target.
> Or as I propose in Bug 1361081, use the Shim to get the button id, to
> clarify dependencies.

I'm sorry I don't really understand the options here.

> But I know we also mentioned removing ourselves from the available UITour
> targets. Do you feel like we should do that now? (I attached a patch that
> does that, in case).

I'm trying to trigger the tour so I can review it.  I think its ok to do that now, but I'd like to get a better sense of it first.

Comment 6

[Julian Descottes [:jdescottes]]

(In reply to Bryan Clark (DevTools PM) [:clarkbw] from comment #5)
> (In reply to Julian Descottes [:jdescottes] from comment #4)
> > We could leave it as is and remain a valid UITour target.
> > Or as I propose in Bug 1361081, use the Shim to get the button id, to
> > clarify dependencies.
> 
> I'm sorry I don't really understand the options here.
> 

If we keep the button as a valid UI Tour target, we can programmatically retrieve the id from our shim. We discarded this option in Bug 1361081, so let's just say one option is to keep the UI Tour target, other option is to remove it.

> > But I know we also mentioned removing ourselves from the available UITour
> > targets. Do you feel like we should do that now? (I attached a patch that
> > does that, in case).
> 
> I'm trying to trigger the tour so I can review it.  I think its ok to do
> that now, but I'd like to get a better sense of it first.

You can simply go to https://www.mozilla.org/en-US/firefox/54.0a2/firstrun/ with any firefox version and you should be able to start it.