Closed Bug 1160663 Opened 4 years ago Closed 4 years ago

Allow hilighting the Pocket button via UITour

Categories

(Firefox :: Tours, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 40
Tracking Status
firefox38.0.5 --- fixed
firefox39 --- fixed
firefox40 --- fixed

People

(Reporter: Dolske, Assigned: Dolske)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch Patch v.1 (obsolete) — Splinter Review
We're not currently planning to have a UITour for Pocket (at least in 38.0.5), but it's trivial to allow hilighting the button so we might as well do so anyway, just in case.
Attachment #8600484 - Flags: review?(MattN+bmo)
Comment on attachment 8600484 [details] [diff] [review]
Patch v.1

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

::: browser/components/uitour/UITour.jsm
@@ +190,5 @@
>          }
>          return loopBrowser.contentDocument.querySelector(".signin-link");
>        },
>      }],
> +    ["pocket", {query: "#pocket-button"}],

If you want a tour to be able to automatically move it (back) to the nav-bar you also need to add the widgetName property.
Attachment #8600484 - Flags: review?(MattN+bmo) → review+
Comment on attachment 8600484 [details] [diff] [review]
Patch v.1

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

Actually, I think you'll also need to update the available targets test or it will fail.
Attached patch Patch v.2 (obsolete) — Splinter Review
Untested test, need to apply 1155521 to check it out.
Attachment #8600484 - Attachment is obsolete: true
Flags: needinfo?(dolske)
Comment on attachment 8600493 [details] [diff] [review]
Patch v.2

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

::: browser/components/uitour/test/browser_UITour_availableTargets.js
@@ +56,5 @@
>          "help",
>          "home",
>          "loop",
>          "devtools",
> +        ...(hasPocket ? ["pocket"] : [])

The test fails because you're missing commas on this line and the similar stuff below
Comment on attachment 8602402 [details] [diff] [review]
Patch v.3

Approval Request Comment
[Feature/regressing bug #]: Pocket product page and snippets
[User impact if declined]: No ability for Mozilla web content (including about:home) to annotate the Pocket button
[Describe test coverage new/current, TreeHerder]: Updated b-c test
[Risks and why]: Low risk UITour change that is isolated to tour code
[String/UUID change made/needed]: None
Attachment #8602402 - Flags: approval-mozilla-release?
Attachment #8602402 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/377431f8f408
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
Attachment #8602402 - Flags: approval-mozilla-release?
Attachment #8602402 - Flags: approval-mozilla-release+
Attachment #8602402 - Flags: approval-mozilla-aurora?
Attachment #8602402 - Flags: approval-mozilla-aurora+
QA Contact: andrei.vaida
You need to log in before you can comment on or make changes to this bug.