1190450 - Tracking Protection fires onSecurityChange with the wrong webProgress.currentURI when the tracking request happens onunload

Status: VERIFIED FIXED

(Core :: Security, defect, P1)

Description

[Brian Grinstead [:bgrins]]

This is a follow up to the investigation in Bug 1185117, specifically Comments 5 and 6.

STR:

Open a new tab to page 1: data:text/html,hi
Enter in this URL to go to page 2: https://bug1185117.bmoattachments.org/attachment.cgi?id=8642472
Press back

Expected:

The tracking shield does not show up on Page 1 after going back

Actual:

The tracking shield shows up on Page 1 after going back

The reason the shield shows up is because there is an onSecurityChange notification that fires with STATE_BLOCKED_TRACKING_CONTENT.  And if you log out the webProgressListener.currentURI or webProgressListener.DOMWindow.location, it's returning Page 1's URI even though the tracking request originated from Page 2.

Comment 1

[Brian Grinstead [:bgrins]]

I'd expect either:

1) no STATE_BLOCKED_TRACKING_CONTENT notification should fire if the page has already navigated away
2) the currentURI and other security info should be set correctly so that consumers can distinguish between this and a legitimate onSecurityChange call from the first page

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

Brian, I can't reproduce this. trackertest.org doesn't seem to exist, is that why the attachment fails to help reproduce this?

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

I can reproduce with the original STR from bug 1185117 though.

Comment 4

[Valentin Gosu [:valentin] (he/him)]

Attachment: TP shield shows up incorrectly when pressing back while loading a page with tracking elements

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

[Tracking Requested - why for this release]:

As we're planning to ship TP in 42 we should definitely uplift the suggested fix to prevent false positives.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/827df7f198cf

Comment 7

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

https://hg.mozilla.org/mozilla-central/rev/827df7f198cf

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Tracking to make sure we don't miss it.
Valentin, could you fill the uplift request to 42? Thanks

Comment 9

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8646902 [details] [diff] [review]
TP shield shows up incorrectly when pressing back while loading a page with tracking elements

Approval Request Comment
[Feature/regressing bug #]:
Always existed.

[User impact if declined]:
TP shield will appear on pages with no tracking content if you click back before a page has finished loading.

[Describe test coverage new/current, TreeHerder]:
None. Tested manually.

[Risks and why]: 
Low risk. This adds a check that the current window, to which we add the TP shield, is the same as the one that opened the channel containing tracking content.

[String/UUID change made/needed]:
None.

Comment 10

[Valentin Gosu [:valentin] (he/him)]

Tim, you mentioned on IRC that it would be easy to add an automated test for this. If you don't have time to do it yourself, could you provide some info about how it would work? Thanks!

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8646902 [details] [diff] [review]
TP shield shows up incorrectly when pressing back while loading a page with tracking elements

TP is a new feature, taking it to polish it.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/0f8540c2a9d9

Comment 13

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Valentin Gosu [:valentin] from comment #10)
> Tim, you mentioned on IRC that it would be easy to add an automated test for
> this. If you don't have time to do it yourself, could you provide some info
> about how it would work? Thanks!

Hmm, iirc the issue wasn't easy to reproduce reliably, but attachment 8642472 [details] might have a way to do that. It just needs to try loading a tracker which didn't work at the time I tried, for some reason. It might be enough to just follow comment #0, load a tab, navigate to the test page, and navigate back. Then check whether the security state indicates a tracker is present.

Comment 14

[Paul Silaghi, QA [:pauly]]

Reproduced couple of times on Nightly 2015-08-03.
Verified fixed 42.0a2 (2015-09-10), 43.0a1 (2015-09-10) Win 7