Closed Bug 1371144 Opened 2 years ago Closed 2 years ago

Should support select `library` with UITour

Categories

(Firefox :: Tours, enhancement, P2)

enhancement

Tracking

()

VERIFIED FIXED
Firefox 56
Tracking Status
firefox55 --- fixed

People

(Reporter: gasolin, Assigned: gasolin)

Details

(Whiteboard: [photon-onboarding])

Attachments

(1 file)

The new onboarding overlay update user tour contains `library tour`, which will use UITour to select library from the photon menu.
Priority: -- → P2
Whiteboard: [photon-onboarding]
Target Milestone: --- → Firefox 57
Flags: qe-verify+
QA Contact: jwilliams
Assignee: nobody → gasolin
Status: NEW → ASSIGNED
Comment on attachment 8876046 [details]
Bug 1371144 - Add the library button as a UITour target.

https://reviewboard.mozilla.org/r/147500/#review151744

::: browser/components/uitour/test/browser_UITour_availableTargets.js:9
(Diff revision 1)
>  var gContentAPI;
>  var gContentWindow;
>  
>  var hasPocket = Services.prefs.getBoolPref("extensions.pocket.enabled");
> -var hasQuit = !Services.prefs.getBoolPref("browser.photon.structure.enabled") ||
> +var isPhoton = Services.prefs.getBoolPref("browser.photon.structure.enabled");
> +var hasQuit = ! isPhoton ||

Nit: remove the space before `isPhoton`
Attachment #8876046 - Flags: review?(MattN+bmo) → review+
thanks!
Keywords: checkin-needed
Comment on attachment 8876046 [details]
Bug 1371144 - Add the library button as a UITour target.

https://reviewboard.mozilla.org/r/147500/#review151786

::: commit-message-f4262:1
(Diff revision 2)
> +Bug 1371144 - Should support s select library with UITour;r=mattN

Actually your commit message also has a problem due to the backticks not being escaped probably. Suggestion:

Bug 1371144 - Add the library button as a UITour target. r=MattN
Keywords: checkin-needed
Target Milestone: Firefox 57 → ---
commit message updated, thanks!
Keywords: checkin-needed
Autoland can't push this until all pending issues in MozReview are marked as resolved.
Keywords: checkin-needed
marked as resolved on mozreview, thanks
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ab78c7663a97
Add the library button as a UITour target. r=MattN
Keywords: checkin-needed
Target Milestone: --- → Firefox 56
https://hg.mozilla.org/mozilla-central/rev/ab78c7663a97
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
How can I verify this fix for QA purposes?
Flags: needinfo?(gasolin)
Justin, there's no UI available now. But once bug 1357046 is landed , you can 


1. set "browser.photon.structure.enabled" preference to `true` in `about:config`
2. In about:newtab, open menu > web developer > webconsole and type:

```
Mozilla.UITour.setHightLight("library")
```

Then you can check if the tool can select to `library` correctly.
Flags: needinfo?(gasolin)
(In reply to Fred Lin [:gasolin] from comment #13)
> ```
> Mozilla.UITour.setHightLight("library")
> ```

That should be:
```
Mozilla.UITour.showHighLight("library")
```
I can see this feature implemented in Latest Nightly 56.0a1 in 64bit, Manjaro Linux following the instruction from comment 13 and comment 15. 

Correct function call is "Mozilla.UITour.showHighlight("library")" with 'l' in "--light" being a lowercase letter.

Build ID 	20170617100144
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:56.0) Gecko/20100101 Firefox/56.0
QA Whiteboard: [bugday-20170614]
This issue has been verified on Win 10 x64.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.