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

Fix pushing an URL to parseURI (AdReport Broken) #127

Merged
merged 2 commits into from Feb 27, 2017

Conversation

tomasko126
Copy link
Member

Fixes #126

@kpeckett Another high-brio bug, could you please take a look at it?

Thanks! :)

Signed-off-by: Tomáš Taro <tomas@getadblock.com>
@tomasko126 tomasko126 changed the title Fix pushing an URL to parseURI Fix pushing an URL to parseURI Jan 19, 2017
@tomasko126 tomasko126 added this to the 1.5.1 milestone Jan 19, 2017
@tomasko126 tomasko126 added bug This is a bug with the extension pr-high This is a high-priority issue. If you want to help out, try one of these if you can! labels Jan 19, 2017
@itskdog itskdog changed the title Fix pushing an URL to parseURI Fix pushing an URL to parseURI (AdReport Broken) Jan 20, 2017
} else {
if (extracted_domains.indexOf(new parseURI(key).hostname) === -1) {
extracted_domains.push(new parseURI(key).hostname);
}
Copy link
Member

Choose a reason for hiding this comment

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

Why is the else clause no longer needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

@kpeckett Because we can't push key to parseURI anymore, since key looks like: 1:|:https://example.com:|:document and that's not a valid URI -> in this case parseURI would throw an exception.

@tomasko126 tomasko126 merged commit 6404791 into master Feb 27, 2017
@tomasko126 tomasko126 deleted the fix-adreport-page branch February 27, 2017 15:42
@tomasko126 tomasko126 moved this from Waiting for review to Merged in Release tracker for 1.5.1 Mar 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This is a bug with the extension pr-high This is a high-priority issue. If you want to help out, try one of these if you can!
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants