Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show better info when no 3rd party resources present. #1627

Closed
wants to merge 14 commits into from

Conversation

cowlicks
Copy link
Contributor

@cowlicks cowlicks commented Sep 1, 2017

closes #1625 #1211

related to #1443

before

Previously on a page with no 3rd party resources, we saw:
pb-no-trackers

And for an extension pages, or a page with no tabId in tabData
pb-extpage
:

after

The popup now looks like this on websites with no 3rd party resources, and for pages with not tabId in tabData:

bg

Before this, the popup was sized to include the "PB has detected..."
text. This would get replaced with the "No 3rd party..." text, but the
popup height would not shrink. This commit fixes that.
@eenblam
Copy link
Contributor

eenblam commented Sep 3, 2017

Fails on a test that tries to follow the "trackers" link. This link no longer appears when visiting eff.org, as "There are no third party resources on this site." is presented instead.

trackers_link = self.driver.find_element_by_link_text("trackers")

@cowlicks
Copy link
Contributor Author

cowlicks commented Sep 4, 2017

@eenblam thank you for looking in to this. I'll fix that test case. And add another one that tests this new functionality explicitly.

@cowlicks
Copy link
Contributor Author

cowlicks commented Sep 5, 2017

Updated to fix the broken test, and add a test.

We didn't have a nice way to load the PB popup for a given tab, so I used some of the functionality @lemnis added (reference: #1516 (comment) ) to add a utility to do this. This code is 660bf60 1773272 and db8fa81

I use this utility to add a selenium testing utility in 6748aee

Fix the broken test in 507fb

Then add a test for the functionality this PR adds in f633157

Note there is a similar function in tests/selenium/cookie_test.py but it has bugs so I didn't use it here. I'll make a separate issue to replace it with this code.

@cowlicks
Copy link
Contributor Author

cowlicks commented Sep 5, 2017

@eenblam I fixed the broken tests. How does it look to you?

eff_url = "https://www.eff.org/privacybadger#faq-What-is-a-third-party-tracker?"
self.wait(lambda driver: driver.current_url == eff_url)
Copy link
Contributor

@eenblam eenblam Sep 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following assertion will never reach a failure state. Here, WebDriverWait is called without any exception handling, so it will raise selenium.common.exceptions.TimeoutException if the two urls do not equal each other before the timeout. So, the test will fail here, and we won't get the nice message from the assertion below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I know :(

I did this to avoid using an explicit wait like time.sleep(2). I guess I could put this whole part in a try/except and fail with the message in the except.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't show tracker message in popup on extension pages.
3 participants