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

Status: VERIFIED FIXED

(Firefox :: Tours, defect, P1)

Description

[Matthew N. [:MattN]]

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.

Comment 1

[Matthew N. [:MattN]]

Attachment: MozReview Request: Bug 1188443 - UITour: Add Tracking Protection "Enable protection" button as a UITour target. r=paolo

Bug 1188443 - UITour: Add Tracking Protection "Enable protection" button as a UITour target. r=paolo

Comment 2

[:Paolo Amadini]

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!

Comment 3

[:Paolo Amadini]

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).

Comment 4

[Matthew N. [:MattN]]

(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.

Comment 5

[Pulsebot]

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

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d0a1ddd01c1e

Comment 7

[Paul Silaghi, QA [:pauly]]

I'm not sure how to enable the tour in order to test this. Could you please advise?

Comment 8

[Paul Silaghi, QA [:pauly]]

Verified fixed FF 42.0a2 (2015-09-10) Win 7