Closed Bug 1068290 Opened 7 years ago Closed 7 years ago

UI Tour: Add ability to highlight New Private Window icon in chrome


(Firefox :: General, defect)

33 Branch
Not set



Firefox 35
Tracking Status
firefox33 --- fixed
firefox34 --- fixed
firefox35 --- fixed


(Reporter: Habber, Assigned: Dolske)




(1 file, 2 obsolete files)

Attached image privatebrowsingicon_tour_mockup.png (obsolete) —
Add ability to highlight New Private Window icon in chrome. For this tour, icon will be highlighted if icon exists in toolbar or menu panel. 

If icon doesn't exist in either because the user has moved it into the palette, step in tour will still be shown, but highlight/icon location will not be indicated. In the future we may need to consider conditional cases, where if an icon is not in toolbar or menu panel, we will omit the step all-together. For the sake of this step and tour, however, we will show to all users.
Flags: firefox-backlog?
Target Milestone: --- → Firefox 33
Blocks: 1071187
Assignee: nobody → dolske
Iteration: --- → 35.2
Points: --- → 2
Flags: firefox-backlog? → firefox-backlog+
OS: Mac OS X → All
Hardware: x86 → All
Attachment #8490356 - Attachment is obsolete: true
Attached patch Patch v.1 (obsolete) — Splinter Review
This is exactly the same as what bug 1045927 added for the Hello button.

Checked that the test passes, and that Mozilla.UITour.showHilight("privateWin") actually works.
Attachment #8494064 - Flags: review?(MattN+bmo)
Comment on attachment 8494064 [details] [diff] [review]
Patch v.1

Review of attachment 8494064 [details] [diff] [review]:

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

Nit: part of using this target map is to allow us to give targets nicer names for API users. With that in mind, I would prefer we don't truncate this target name and use "privateWindow". I realize the indentation will differ for this line but I think that's less of an issue.
Attachment #8494064 - Flags: review?(MattN+bmo) → review+
Flags: qe-verify?
Attached patch Patch v.2Splinter Review
Attachment #8494064 - Attachment is obsolete: true
Keywords: checkin-needed
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.
Flags: qe-verify? → qe-verify-
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Blocks: fx10
No longer blocks: fx10
Comment on attachment 8494142 [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.)
Attachment #8494142 - Flags: approval-mozilla-beta+
Target Milestone: Firefox 33 → Firefox 35
Landed in alder:
Whiteboard: [fixed-alder]
You need to log in before you can comment on or make changes to this bug.