1072036 - UI Tour: Add ability to highlight new privacy button

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Justin Dolske [:Dolske]]

If the user adds the new bug 1069300 button (using the new UITour feature from bug 1071238), it seems likely we'll want to then immediately hilight it. Trivial patch, so let's go ahead and do that.

Comment 1

[Justin Dolske [:Dolske]]

Attachment: Patch v.1

Basically the same as bug 1068290.

Comment 2

[Justin Dolske [:Dolske]]

Comment on attachment 8494155 [details] [diff] [review]
Patch v.1

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

Might as well get review, but I don't intend to land this until bug bug 1069300 is at least further along.

Comment 3

[Matthew N. [:MattN]]

Comment on attachment 8494155 [details] [diff] [review]
Patch v.1

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

::: browser/modules/UITour.jsm
@@ +97,5 @@
>      }],
>      ["help",        {query: "#PanelUI-help"}],
>      ["home",        {query: "#home-button"}],
>      ["loop",        {query: "#loop-call-button"}],
> +    ["panicButton", {query: "#panic-button"}],

Nit: the "Button" suffix seems somewhat out of place given that none of the other buttons have this suffix.

Comment 4

[Robert Kaiser]

I think that as long as it's only the ability and no tour that actually highlights it has been implemented, we cannot manually test it.

Comment 5

[Matthew N. [:MattN]]

Comment on attachment 8494155 [details] [diff] [review]
Patch v.1

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

::: browser/modules/UITour.jsm
@@ +97,5 @@
>      }],
>      ["help",        {query: "#PanelUI-help"}],
>      ["home",        {query: "#home-button"}],
>      ["loop",        {query: "#loop-call-button"}],
> +    ["panicButton", {query: "#panic-button"}],

Also, can you change this to the newer name "forget"?

Comment 6

[Justin Dolske [:Dolske]]

Attachment: Patch v.2

Changed the name from "panicButton" to "forget", to address previous comment and to make the name we expose to the UITour API consistent with what the feature is called.

https://hg.mozilla.org/integration/fx-team/rev/83f3c381d339

Comment 7

[Justin Dolske [:Dolske]]

(In reply to Matthew N. [:MattN] from comment #5)

> Also, can you change this to the newer name "forget"?

Ha, I made and landed that change 8 seconds before your comment. :)

Comment 8

[Matthew N. [:MattN]]

I figured that since I got the mention from #developers just a few seconds after I commented. My other theory was that you have a time machine.

Comment 9

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/83f3c381d339

Comment 10

[Justin Dolske [:Dolske]]

Comment on attachment 8496221 [details] [diff] [review]
Patch v.2

a+ for beta as this bug will be landing in the 33.x anniversary release. (It's already on Nightly + Aurora, so only needs a beta landing at this point.)

Comment 11

[Blair McBride [:Unfocused] (UNAVAILABLE)]

https://hg.mozilla.org/releases/mozilla-beta/rev/d37b92959827

Comment 12

[Justin Dolske [:Dolske]]

Landed in alder: https://hg.mozilla.org/projects/alder/rev/62fee88552f8

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/62fee88552f8