Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code
Issues
All Issues

Side by Side Diff: chrome/content/tests/popupBlocker.js

Issue 29331996: Issue 3244 - Pop-up blocker unit tests broken in E10S mode (Closed)
Patch Set: Created Dec. 4, 2015, 8:56 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View unified diff | Download patch
« chrome/content/common.js ('K') | « chrome/content/common.js ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 (function() 1 (function()
2 { 2 {
3 let tabs = SDK.require("sdk/tabs");
4 let modelFor = SDK.require("sdk/model/core").modelFor;
Erik 2015/12/04 20:58:33 oh I should remove this.
3 let server = null; 5 let server = null;
4 let wnd = null;
5 let tab = null; 6 let tab = null;
6 7
7 module("Pop-up blocker", { 8 module("Pop-up blocker", {
8 setup: function() 9 beforeEach: function()
9 { 10 {
10 prepareFilterComponents.call(this, true); 11 prepareFilterComponents.call(this, true);
11 preparePrefs.call(this); 12 preparePrefs.call(this);
12 13
13 server = new nsHttpServer(); 14 server = new nsHttpServer();
14 server.start(1234); 15 server.start(1234);
15 16
17 // '/test' serves an html page with a single link
16 server.registerPathHandler("/test", function(metadata, response) 18 server.registerPathHandler("/test", function(metadata, response)
17 { 19 {
18 response.setStatusLine("1.1", "200", "OK"); 20 response.setStatusLine("1.1", "200", "OK");
19 response.setHeader("Content-Type", "text/html; charset=utf-8"); 21 response.setHeader("Content-Type", "text/html; charset=utf-8");
20 22
21 let body = 23 let body =
22 '<body onload="document.dispatchEvent(new CustomEvent(\'abp:frameready \', {bubbles: true}));">' + 24 '<body>' +
23 '<a id="link" href="/redirect" target="_blank">link</a>' + 25 '<a id="link" href="/redirect" target="_blank">link</a>' +
24 '</body>'; 26 '</body>';
25 response.bodyOutputStream.write(body, body.length); 27 response.bodyOutputStream.write(body, body.length);
26 }); 28 });
29
30 // redirects '/redirect' to '/target'
27 server.registerPathHandler("/redirect", function(metadata, response) 31 server.registerPathHandler("/redirect", function(metadata, response)
28 { 32 {
29 response.setStatusLine("1.1", "302", "Moved Temporarily"); 33 response.setStatusLine("1.1", "302", "Moved Temporarily");
30 response.setHeader("Location", "http://127.0.0.1:1234/target"); 34 response.setHeader("Location", "http://127.0.0.1:1234/target");
31 }); 35 });
36
37 // '/target' serves an html page with 'OK' message
32 server.registerPathHandler("/target", function(metadata, response) 38 server.registerPathHandler("/target", function(metadata, response)
33 { 39 {
34 response.setHeader("Content-Type", "text/html; charset=utf-8"); 40 response.setHeader("Content-Type", "text/html; charset=utf-8");
35 41
36 let body = '<html><body>OK</body></html>'; 42 let body = '<html><body>OK</body></html>';
37 response.bodyOutputStream.write(body, body.length); 43 response.bodyOutputStream.write(body, body.length);
38 }); 44 });
39 45
40 wnd = UI.currentWindow; 46 tabs.open({
41 tab = wnd.gBrowser.loadOneTab("http://127.0.0.1:1234/test", {inBackground: false}); 47 url: "http://127.0.0.1:1234/test",
42 wnd.gBrowser.getBrowserForTab(tab).addEventListener("abp:frameready", func tion(event) 48 inBackground: false,
43 { 49 onReady: function(aTab)
44 start(); 50 {
45 }, false, true); 51 tab = aTab;
52 var worker = tab.attach({
53 contentScript: "(" + function()
54 {
55 if (document.getElementById("link"))
56 self.port.emit("done");
57 } + ")()"
58 });
Wladimir Palant 2015/12/14 11:28:53 I don't think that clicking the link should be par
59
60 worker.port.once("done", function()
61 {
62 start();
63 })
Wladimir Palant 2015/12/14 11:28:53 Nit: No need to wrap the callback here, this will
64 }
65 });
46 66
47 stop(); 67 stop();
48 }, 68 },
49 teardown: function() 69 afterEach: function()
50 { 70 {
51 restoreFilterComponents.call(this); 71 restoreFilterComponents.call(this);
52 restorePrefs.call(this); 72 restorePrefs.call(this);
53 73
54 stop(); 74 stop();
55 server.stop(function() 75 server.stop(function()
56 { 76 {
57 wnd.gBrowser.removeTab(tab); 77 tab.close(function()
58 78 {
59 server = null; 79 server = null;
60 frame = null; 80 start();
61 81 });
62 start();
63 }); 82 });
64 } 83 }
65 }); 84 });
66 85
67 let tests = [ 86 let tests = [
68 ["||127.0.0.1:1234/target$popup", false], 87 // filter says '/target' as a popup should be blocked
88 // expect no opened tab
Wladimir Palant 2015/12/14 11:28:52 Frankly, I think that this is overdocumenting. I w
Erik 2015/12/29 22:49:40 We can leave this for another issue, I'll take it
89 //["||127.0.0.1:1234/target$popup", false],
Wladimir Palant 2015/12/14 11:28:53 Why is this test commented out, isn't it working c
Erik 2015/12/29 22:49:40 oh I didn't notice that I did that, iirc I comment
90 // filter says '/target' as a subdoc should be blocked
91 // expect to find OK message in opened tab (b/c popup is not blocked)
69 ["||127.0.0.1:1234/target$~subdocument", true], 92 ["||127.0.0.1:1234/target$~subdocument", true],
93 // filter says '/target' as a popup should be blocked from domain 127.0.0.1
94 // expect no opened tab
70 ["||127.0.0.1:1234/target$popup,domain=127.0.0.1", false], 95 ["||127.0.0.1:1234/target$popup,domain=127.0.0.1", false],
96 // filter says '/target' as a popup should be blocked from domain 128.0.0.1
97 // expect to find OK message in opened tab (b/c popup is not blocked)
71 ["||127.0.0.1:1234/target$popup,domain=128.0.0.1", true], 98 ["||127.0.0.1:1234/target$popup,domain=128.0.0.1", true],
99 // filter says '/redirect' as a popup should be blocked
100 // expect no opened tab
72 ["||127.0.0.1:1234/redirect$popup", false], 101 ["||127.0.0.1:1234/redirect$popup", false],
102 // filter says '/redirect' as a subdocument should be blocked
103 // expect to find OK message in opened tab (b/c popup is not blocked)
73 ["||127.0.0.1:1234/redirect$~subdocument", true], 104 ["||127.0.0.1:1234/redirect$~subdocument", true],
105 // filter says '/redirect' as a popup should be blocked from domain 127.0.0. 1
106 // expect no opened tab
74 ["||127.0.0.1:1234/redirect$popup,domain=127.0.0.1", false], 107 ["||127.0.0.1:1234/redirect$popup,domain=127.0.0.1", false],
108 // filter says '/redirect' as a popup should be blocked from domain 128.0.0. 1
109 // expect to find OK message in opened tab (b/c popup is not blocked)
75 ["||127.0.0.1:1234/redirect$popup,domain=128.0.0.1", true], 110 ["||127.0.0.1:1234/redirect$popup,domain=128.0.0.1", true],
76 ]; 111 ];
112 var testCount = 0;
Wladimir Palant 2015/12/14 11:28:52 This variable seems unused.
77 113
78 function runTest(filter, result) 114 function runTest(filter, result)
79 { 115 {
116 tabs.off("ready", onTabOpen);
Wladimir Palant 2015/12/14 11:28:53 This seems pointless, why would there be an existi
Erik 2015/12/29 22:49:40 This should be in `onTabOpen`.
80 FilterStorage.addFilter(filter); 117 FilterStorage.addFilter(filter);
81 118
82 let successful = false; 119 let successful = false;
83 120
84 function onTabOpen(event) 121 function onTabOpen(tab) {
Wladimir Palant 2015/12/14 11:28:52 Style nit: Bracket on the next line please.
85 { 122 // link in '/test' was clicked
123 tab.on("close", onTabClose);
86 window.clearTimeout(timeout); 124 window.clearTimeout(timeout);
87 wnd.gBrowser.tabContainer.removeEventListener("TabOpen", onTabOpen, false) ;
88 125
89 let tab = event.target; 126 var worker = tab.attach({
90 let browser = wnd.gBrowser.getBrowserForTab(tab); 127 contentScriptWhen: "ready",
91 Utils.runAsync(function() 128 contentScript: "self.port.emit('done', document.body.innerHTML.toString( ));"
Wladimir Palant 2015/12/14 11:28:52 No point calling toString() here - innerHTML is al
Erik 2015/12/29 22:49:40 good point.
129 });
130
131 worker.port.once("done", function(bodyText)
92 { 132 {
93 browser.contentWindow.addEventListener("load", function(event) 133 if (bodyText.indexOf("OK") >= 0)
94 { 134 successful = true;
95 if (browser.contentDocument.body.textContent.indexOf("OK") >= 0)
96 successful = true;
97 135
98 browser.contentWindow.close(); 136 // pop-up was not blocked so close it
99 }, false); 137 tab.close();
100 }); 138 });
101 } 139 }
140 tabs.on("ready", onTabOpen);
102 141
103 function onTabClose(event) 142 function onTabClose(tab)
104 { 143 {
105 wnd.gBrowser.tabContainer.removeEventListener("TabClose", onTabClose, fals e); 144 tabs.off("ready", onTabOpen);
145 if (tab)
146 tab.off("close", onTabClose);
147
106 ok(result == successful, "Opening tab with filter " + filter.text); 148 ok(result == successful, "Opening tab with filter " + filter.text);
149
107 var keys = []; 150 var keys = [];
108 for (let key in defaultMatcher.blacklist.keywordByFilter) 151 for (let key in defaultMatcher.blacklist.keywordByFilter)
109 keys.push(key); 152 keys.push(key);
Wladimir Palant 2015/12/14 11:28:52 I have no idea why this code is here, it isn't bei
Erik 2015/12/29 22:49:40 alright it doesn't seem to be used to me either.
110 153
111 FilterStorage.removeFilter(filter); 154 FilterStorage.removeFilter(filter);
155
112 start(); 156 start();
113 } 157 }
114 158
115 wnd.gBrowser.tabContainer.addEventListener("TabOpen", onTabOpen, false); 159 // In case the tab isn't opened
116 wnd.gBrowser.tabContainer.addEventListener("TabClose", onTabClose, false); 160 var timeout = window.setTimeout(function()
117 let timeout = window.setTimeout(onTabClose, 1000); // In case the tab isn 't opened 161 {
162 onTabClose();
163 }, 1000);
Wladimir Palant 2015/12/14 11:28:52 Nit: no point wrapping the callback here either. T
118 164
119 wnd.gBrowser.getBrowserForTab(tab).contentDocument.getElementById("link").cl ick(); 165 // click the link in the '/test' tab opened before the test
166 var worker = tab.attach({
167 contentScriptWhen: "ready",
168 contentScriptOptions: {
169 filter: filter.toString()
170 },
Wladimir Palant 2015/12/14 11:28:52 The content script doesn't need any options...
171 contentScript: "(" + function()
172 {
173 document.getElementById('link').click();
174 } + ")()"
175 //contentScript: "document.getElementById('link').click();"
Wladimir Palant 2015/12/14 11:28:52 Please remove this line.
176 });
120 } 177 }
121 178
179 // create async qunit tests
Wladimir Palant 2015/12/14 11:28:52 This is stating the obvious again.
122 for (let [filter, result] of tests) 180 for (let [filter, result] of tests)
123 asyncTest(filter, runTest.bind(null, Filter.fromText(filter), result)); 181 asyncTest(filter, runTest.bind(null, Filter.fromText(filter), result));
124 })(); 182 })();
OLDNEW
« chrome/content/common.js ('K') | « chrome/content/common.js ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld