Closed Bug 1188443 Opened 5 years ago Closed 5 years ago

Add the enable TP for this site button as a UITour target

Categories

(Firefox :: Tours, defect, P1)

defect
Points:
2

Tracking

()

VERIFIED FIXED
Firefox 42
Iteration:
42.3 - Aug 10
Tracking Status
firefox42 --- verified

People

(Reporter: MattN, Assigned: MattN)

References

Details

(Whiteboard: [fxprivacy] [campaign])

Attachments

(1 file)

If the user clicks the disable button during the tour, we may want to anchor an info panel on the enable button so we should add a target for it.
Flags: qe-verify+
Flags: firefox-backlog+
Priority: -- → P1
QA Contact: mwobensmith
Bug 1188443 - UITour: Add Tracking Protection "Enable protection" button as a UITour target. r=paolo
Attachment #8644065 - Flags: review?(paolo.mozmail)
Comment on attachment 8644065 [details]
MozReview Request: Bug 1188443 - UITour: Add Tracking Protection "Enable protection" button as a UITour target. r=paolo

https://reviewboard.mozilla.org/r/15209/#review13659

Ship It!
Attachment #8644065 - Flags: review?(paolo.mozmail) → review+
Patch looks good to me, I just have one question as I'm not familiar with the latest Tracking Protection UI tour design (not sure if it's documented somewhere).

From what I understand, the page can query for the available targets to know whether TP is enabled or not on that page, then show a different message based on this. This only works while the Control Center is open, though, so the page cannot know it beforehand, without relying on potentially inaccurate hints like whether a fake tracker load completed, which can be influenced by external factors as well.

Wouldn't it be more robust if the page cold query for the TP state separately at any time with a different configuration query API? We could then use a single target for all the variants of the button (we could even just check which particular button is visible at the moment in the getter, without other logic).
(In reply to :Paolo Amadini from comment #3)
> From what I understand, the page can query for the available targets to know
> whether TP is enabled or not on that page, then show a different message
> based on this. This only works while the Control Center is open, though, so
> the page cannot know it beforehand, without relying on potentially
> inaccurate hints like whether a fake tracker load completed, which can be
> influenced by external factors as well.

The page could detect a reload on step 3 where the tracking content was blocked initially but then loaded to handle this case. It's not as fragile as relying solely on whether the tracker loaded or not.

> Wouldn't it be more robust if the page cold query for the TP state
> separately at any time with a different configuration query API? We could
> then use a single target for all the variants of the button (we could even
> just check which particular button is visible at the moment in the getter,
> without other logic).

Yeah, we could but I'd rather not add a specific API for this if we can avoid it and this could possibly be seen as more easily leaking to UITour origins whether PB is enabled. We can change this later if we need to for the tour.
https://hg.mozilla.org/mozilla-central/rev/d0a1ddd01c1e
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
I'm not sure how to enable the tour in order to test this. Could you please advise?
Flags: needinfo?(MattN+bmo)
Verified fixed FF 42.0a2 (2015-09-10) Win 7
Status: RESOLVED → VERIFIED
Flags: needinfo?(MattN+bmo)
You need to log in before you can comment on or make changes to this bug.