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 XML document rendering in Firefox #1954

Merged
merged 2 commits into from
Apr 19, 2018
Merged

Conversation

ghostwords
Copy link
Member

@ghostwords ghostwords commented Apr 9, 2018

Fixes #1949.

This only seems to happen in Firefox. Does Chrome handle the checks we add in this PR internally?

Sample XML document: https://sourceforge.net/sitemap.xml
Sample XHTML document: http://tieba.baidu.com/mo/q---2799060AC2198378C599200012D229ED%3AFG%3D1--1-3-0--2--wapp_1440757687244_809/m?kw=%E9%9A%90%E6%9D%80&lp=1030

This seems like a good thing to have automated tests for, but I haven't yet figured out how to detect the difference between a pretty-rendered XML document in Firefox and a non-pretty-thanks-to-an-injected-script-tag version of the same with Selenium. The problem seems to be that Firefox handles its pretty rendering HTML specially; it's not part of the document (doesn't seem to be even inspectable by dev tools).

Here is my (non-working) test attempt:
diff --git a/tests/selenium/breakage_test.py b/tests/selenium/breakage_test.py
index 33a3743..db86fd8 100644
--- a/tests/selenium/breakage_test.py
+++ b/tests/selenium/breakage_test.py
@@ -1,14 +1,16 @@
 #!/usr/bin/env python
 # -*- coding: UTF-8 -*-
 
+import os
 import unittest
+
 import pbtest
+
 from selenium.webdriver.support.ui import WebDriverWait
 from selenium.webdriver.support import expected_conditions as EC
-import re
 
 
-class Test(pbtest.PBSeleniumTest):
+class BreakageTest(pbtest.PBSeleniumTest):
     """Make sure the extension doesn't break common sites and use cases.
     e.g. we should be able to load a website, search on Google.
     TODO: Add tests to simulate most common web use cases:
@@ -27,5 +29,14 @@ class Test(pbtest.PBSeleniumTest):
         qry_el.submit()
         WebDriverWait(self.driver, 10).until(EC.title_contains("EFF"))
 
+    # https://github.com/EFForg/privacybadger/issues/1949
+    def test_xml_rendering(self):
+        xml_fixture_path = os.path.join(
+            pbtest.get_git_root(), 'tests', 'fixtures', 'xml_document.xml')
+        self.load_url('file://' + xml_fixture_path)
+        page_contents = self.js("return document.outerHTML;")
+        self.assertEqual(page_contents, '<root>Test</root>',
+            "XML document's contents should have been left alone.")
+
 if __name__ == "__main__":
     unittest.main()
$ cat tests/fixtures/xml_document.xml 
<root>Test</root>

@ghostwords ghostwords requested a review from bcyphers April 9, 2018 18:48
@ghostwords
Copy link
Member Author

ghostwords commented Apr 9, 2018

This PR also adds a check to avoid injecting the canvas fingerprinting detection script on first-party frames. This check already exists when receiving canvas reports, so why not avoid modifying page objects unless we need to do it. This may help with some canvas-related site breakages (if the canvas is first-party, the breakage will go away).

Oops, I goofed, 7f7ff1d (rebased out) doesn't work since we need to compare script URL to frame URL and we don't have script URL at the time of injection yet.

@ghostwords ghostwords force-pushed the fix-xml-docs-in-firefox branch from e1304a9 to f6db2db Compare April 9, 2018 19:28
@bcyphers
Copy link
Contributor

This looks good. I tried for a while, but could not find any way for selenium to differentiate between XML that is rendered as a document tree and the broken pseudo-HTML that we get in the error case. Chrome seems to rewrite the document itself, but firefox leaves the source exactly the same.

Anyway -- it seems like more trouble than its worth to write a test for this right now.

@ghostwords ghostwords merged commit f6db2db into master Apr 19, 2018
ghostwords added a commit that referenced this pull request Apr 19, 2018
Fix XML document rendering in Firefox.
@ghostwords ghostwords deleted the fix-xml-docs-in-firefox branch April 19, 2018 14:42
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.

Broken XML rendering in Firefox
2 participants