Closed
Bug 1190450
Opened 9 years ago
Closed 9 years ago
Tracking Protection fires onSecurityChange with the wrong webProgress.currentURI when the tracking request happens onunload
Categories
(Core :: Security, defect, P1)
Core
Security
Tracking
()
People
(Reporter: bgrins, Assigned: valentin)
References
(Depends on 1 open bug)
Details
(Whiteboard: [fxprivacy])
Attachments
(1 file)
(deleted),
patch
|
mcmanus
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•9 years ago
|
||
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
Updated•9 years ago
|
Flags: qe-verify?
Flags: firefox-backlog+
Priority: -- → P2
Whiteboard: [fxprivacy]
Reporter | ||
Updated•9 years ago
|
Flags: qe-verify? → qe-verify+
Updated•9 years ago
|
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
Iteration: --- → 43.1 - Aug 24
Priority: P2 → P1
Comment 2•9 years ago
|
||
Brian, I can't reproduce this. trackertest.org doesn't seem to exist, is that why the attachment fails to help reproduce this?
Flags: needinfo?(bgrinstead)
Comment 3•9 years ago
|
||
I can reproduce with the original STR from bug 1185117 though.
Assignee | ||
Comment 4•9 years ago
|
||
Attachment #8646902 -
Flags: review?(mcmanus)
Assignee | ||
Updated•9 years ago
|
Assignee: ttaubert → valentin.gosu
Comment 5•9 years ago
|
||
[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.
status-firefox40:
--- → wontfix
status-firefox41:
--- → wontfix
status-firefox42:
--- → affected
status-firefox43:
--- → affected
tracking-firefox42:
--- → ?
Flags: needinfo?(bgrinstead)
Updated•9 years ago
|
Attachment #8646902 -
Flags: review?(mcmanus) → review+
Updated•9 years ago
|
QA Contact: mwobensmith
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Comment 8•9 years ago
|
||
Tracking to make sure we don't miss it.
Valentin, could you fill the uplift request to 42? Thanks
Flags: needinfo?(valentin.gosu)
Assignee | ||
Comment 9•9 years ago
|
||
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.
Flags: needinfo?(valentin.gosu)
Attachment #8646902 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 10•9 years ago
|
||
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!
Flags: needinfo?(ttaubert)
Comment 11•9 years ago
|
||
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.
Attachment #8646902 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 12•9 years ago
|
||
Comment 13•9 years ago
|
||
(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.
Flags: needinfo?(ttaubert)
Comment 14•9 years ago
|
||
Reproduced couple of times on Nightly 2015-08-03.
Verified fixed 42.0a2 (2015-09-10), 43.0a1 (2015-09-10) Win 7
You need to log in
before you can comment on or make changes to this bug.
Description
•