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

Delta Between Two Patch Sets: chrome/content/tests/popupBlocker.js

Issue 29331996: Issue 3244 - Pop-up blocker unit tests broken in E10S mode (Closed)
Left Patch Set: Created Dec. 4, 2015, 8:56 p.m.
Right Patch Set: Issue 3244 - Pop-up blocker unit tests broken in E10S mode Created Feb. 4, 2016, 11:29 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« no previous file with change/comment | « chrome/content/common.js ('k') | no next file » | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 (function() 1 (function()
2 { 2 {
3 let tabs = SDK.require("sdk/tabs"); 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.
5 let server = null; 4 let server = null;
6 let tab = null; 5 let tab = null;
7 6
8 module("Pop-up blocker", { 7 module("Pop-up blocker", {
9 beforeEach: function() 8 beforeEach: function()
10 { 9 {
11 prepareFilterComponents.call(this, true); 10 prepareFilterComponents.call(this, true);
12 preparePrefs.call(this); 11 preparePrefs.call(this);
13 12
14 server = new nsHttpServer(); 13 server = new nsHttpServer();
(...skipping 27 matching lines...) Expand all
42 let body = '<html><body>OK</body></html>'; 41 let body = '<html><body>OK</body></html>';
43 response.bodyOutputStream.write(body, body.length); 42 response.bodyOutputStream.write(body, body.length);
44 }); 43 });
45 44
46 tabs.open({ 45 tabs.open({
47 url: "http://127.0.0.1:1234/test", 46 url: "http://127.0.0.1:1234/test",
48 inBackground: false, 47 inBackground: false,
49 onReady: function(aTab) 48 onReady: function(aTab)
50 { 49 {
51 tab = aTab; 50 tab = aTab;
52 var worker = tab.attach({ 51 start();
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 } 52 }
65 }); 53 });
66 54
67 stop(); 55 stop();
68 }, 56 },
69 afterEach: function() 57 afterEach: function()
70 { 58 {
71 restoreFilterComponents.call(this); 59 restoreFilterComponents.call(this);
72 restorePrefs.call(this); 60 restorePrefs.call(this);
73 61
74 stop(); 62 stop();
75 server.stop(function() 63 server.stop(function()
76 { 64 {
77 tab.close(function() 65 tab.close(function()
78 { 66 {
79 server = null; 67 server = null;
80 start(); 68 start();
81 }); 69 });
82 }); 70 });
83 } 71 }
84 }); 72 });
85 73
86 let tests = [ 74 let tests = [
87 // filter says '/target' as a popup should be blocked 75 ["||127.0.0.1:1234/target$popup", false],
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)
92 ["||127.0.0.1:1234/target$~subdocument", true], 76 ["||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
95 ["||127.0.0.1:1234/target$popup,domain=127.0.0.1", false], 77 ["||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)
98 ["||127.0.0.1:1234/target$popup,domain=128.0.0.1", true], 78 ["||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
101 ["||127.0.0.1:1234/redirect$popup", false], 79 ["||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)
104 ["||127.0.0.1:1234/redirect$~subdocument", true], 80 ["||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
107 ["||127.0.0.1:1234/redirect$popup,domain=127.0.0.1", false], 81 ["||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)
110 ["||127.0.0.1:1234/redirect$popup,domain=128.0.0.1", true], 82 ["||127.0.0.1:1234/redirect$popup,domain=128.0.0.1", true],
111 ]; 83 ];
112 var testCount = 0;
Wladimir Palant 2015/12/14 11:28:52 This variable seems unused.
113 84
114 function runTest(filter, result) 85 function runTest(filter, result)
115 { 86 {
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`.
117 FilterStorage.addFilter(filter); 87 FilterStorage.addFilter(filter);
118 88
119 let successful = false; 89 let successful = false;
120 90
121 function onTabOpen(tab) { 91 function onTabOpen(tab)
Wladimir Palant 2015/12/14 11:28:52 Style nit: Bracket on the next line please.
92 {
93 tabs.off("ready", onTabOpen);
94
122 // link in '/test' was clicked 95 // link in '/test' was clicked
123 tab.on("close", onTabClose); 96 tab.on("close", onTabClose);
124 window.clearTimeout(timeout); 97 window.clearTimeout(timeout);
125 98
126 var worker = tab.attach({ 99 var worker = tab.attach({
127 contentScriptWhen: "ready", 100 contentScriptWhen: "ready",
128 contentScript: "self.port.emit('done', document.body.innerHTML.toString( ));" 101 contentScript: "self.port.emit('done', document.body.textContent);"
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 }); 102 });
130 103
131 worker.port.once("done", function(bodyText) 104 worker.port.once("done", function(bodyText)
132 { 105 {
133 if (bodyText.indexOf("OK") >= 0) 106 if (bodyText.indexOf("OK") >= 0)
134 successful = true; 107 successful = true;
135 108
136 // pop-up was not blocked so close it 109 // pop-up was not blocked so close it
137 tab.close(); 110 tab.close();
138 }); 111 });
139 } 112 }
140 tabs.on("ready", onTabOpen); 113 tabs.on("ready", onTabOpen);
141 114
142 function onTabClose(tab) 115 function onTabClose(tab)
143 { 116 {
144 tabs.off("ready", onTabOpen); 117 tabs.off("ready", onTabOpen);
145 if (tab) 118 if (tab)
146 tab.off("close", onTabClose); 119 tab.off("close", onTabClose);
147 120
148 ok(result == successful, "Opening tab with filter " + filter.text); 121 ok(result == successful, "Opening tab with filter " + filter.text);
149 122
150 var keys = [];
151 for (let key in defaultMatcher.blacklist.keywordByFilter)
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.
153
154 FilterStorage.removeFilter(filter); 123 FilterStorage.removeFilter(filter);
155 124
156 start(); 125 start();
157 } 126 }
158 127
159 // In case the tab isn't opened 128 // In case the tab isn't opened
160 var timeout = window.setTimeout(function() 129 let timeout = window.setTimeout(onTabClose, 1000, null);
161 {
162 onTabClose();
163 }, 1000);
Wladimir Palant 2015/12/14 11:28:52 Nit: no point wrapping the callback here either. T
164 130
165 // click the link in the '/test' tab opened before the test 131 // click the link in the '/test' tab opened before the test
166 var worker = tab.attach({ 132 var worker = tab.attach({
167 contentScriptWhen: "ready", 133 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() 134 contentScript: "(" + function()
172 { 135 {
173 document.getElementById('link').click(); 136 document.getElementById('link').click();
174 } + ")()" 137 } + ")()"
175 //contentScript: "document.getElementById('link').click();"
Wladimir Palant 2015/12/14 11:28:52 Please remove this line.
176 }); 138 });
177 } 139 }
178 140
179 // create async qunit tests
Wladimir Palant 2015/12/14 11:28:52 This is stating the obvious again.
180 for (let [filter, result] of tests) 141 for (let [filter, result] of tests)
181 asyncTest(filter, runTest.bind(null, Filter.fromText(filter), result)); 142 asyncTest(filter, runTest.bind(null, Filter.fromText(filter), result));
182 })(); 143 })();
LEFTRIGHT
« chrome/content/common.js ('k') | no next file » | Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Toggle Comments ('s')

Powered by Google App Engine
This is Rietveld