Skip to content

Commit 7ed6287

Browse files
committed
Context menu bugfix and refactoring
- some rulesets rewrite https -> https, stop breaking those - make populate_menu less byzantine - to facilitate that, lazily store some DOM state in ApplicableList instances
1 parent 3ffb8e6 commit 7ed6287

4 files changed

Lines changed: 93 additions & 82 deletions

File tree

src/chrome/content/code/ApplicableList.js

Lines changed: 75 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -4,35 +4,43 @@
44

55
serial_number = 0
66

7-
function ApplicableList(logger, home) {
8-
this.home = home.baseURIObject.spec; // what doc we're housekeeping for
7+
function ApplicableList(logger, doc, domWin) {
8+
this.domWin = domWin;
9+
this.home = doc.baseURIObject.spec; // what doc we're housekeeping for
910
this.log = logger;
1011
this.active = {};
1112
this.inactive = {};
1213
this.moot={}; // rulesets that might be applicable but uris are already https
14+
this.all={}; // active + inactive + moot
1315
serial_number += 1;
1416
this.serial = serial_number;
1517
this.log(DBUG,"Alist serial #" + this.serial + " for " + this.home);
1618
// The base URI of the dom tends to be loaded from some /other/
1719
// ApplicableList, so pretend we're loading it from here.
18-
HTTPSEverywhere.instance.https_rules.rewrittenURI(this, home.baseURIObject);
20+
HTTPSEverywhere.instance.https_rules.rewrittenURI(this, doc.baseURIObject);
1921
};
2022

2123
ApplicableList.prototype = {
2224

2325
active_rule: function(ruleset) {
24-
this.log(INFO,"active rule " + ruleset.name +" in "+ this.home);
26+
this.log(INFO,"active rule " + ruleset.name +" in "+ this.home +" -> " +
27+
this.domWin.document.baseURIObject.spec+ " serial " + this.serial);
2528
this.active[ruleset.name] = ruleset;
29+
this.all[ruleset.name] = ruleset;
2630
},
2731

2832
inactive_rule: function(ruleset) {
29-
this.log(INFO,"inactive rule " + ruleset.name +" in "+ this.home);
33+
34+
this.log(INFO,"inactive rule " + ruleset.name +" in "+ this.home +" -> " +
35+
this.domWin.document.baseURIObject.spec+ " serial " + this.serial);
3036
this.inactive[ruleset.name] = ruleset;
37+
this.all[ruleset.name] = ruleset;
3138
},
3239

3340
moot_rule: function(ruleset) {
34-
this.log(INFO,"moot rule " + ruleset.name +" in "+ this.home);
41+
this.log(INFO,"moot rule " + ruleset.name +" in "+ this.home + " serial " + this.serial);
3542
this.moot[ruleset.name] = ruleset;
43+
this.all[ruleset.name] = ruleset;
3644
},
3745

3846
dom_handler: function(operation,key,data,src,dst) {
@@ -43,13 +51,14 @@ ApplicableList.prototype = {
4351

4452
populate_menu: function(document) {
4553
this.log(DBUG, "populating using alist #" + this.serial);
54+
this.document = document;
4655

4756
// get the menu popup
48-
var menupopup = document.getElementById('https-everywhere-context');
57+
this.menupopup = document.getElementById('https-everywhere-context');
4958

5059
// empty it all of its menuitems
51-
while(menupopup.firstChild) {
52-
menupopup.removeChild(menupopup.firstChild);
60+
while(this.menupopup.firstChild) {
61+
this.menupopup.removeChild(this.menupopup.firstChild);
5362
}
5463

5564
// add the label at the top
@@ -64,105 +73,100 @@ ApplicableList.prototype = {
6473
label.setAttribute('label', '(No Rules for This Page)');
6574
}
6675
label.setAttribute('command', 'https-everywhere-menuitem-preferences');
67-
menupopup.appendChild(label);
76+
this.menupopup.appendChild(label);
6877

6978
// create a commandset if it doesn't already exist
70-
var commandset = document.getElementById('https-everywhere-commandset');
71-
if(!commandset) {
72-
commandset = document.createElement('commandset');
73-
commandset.setAttribute('id', 'https-everywhere-commandset');
79+
this.commandset = document.getElementById('https-everywhere-commandset');
80+
if(!this.commandset) {
81+
this.commandset = document.createElement('commandset');
82+
this.commandset.setAttribute('id', 'https-everywhere-commandset');
7483
var button = document.getElementById('https-everywhere-button');
75-
button.appendChild(commandset);
84+
button.appendChild(this.commandset);
7685
} else {
7786
// empty commandset
78-
while(commandset.firstChild) {
79-
commandset.removeChild(commandset.firstChild);
80-
}
87+
while(this.commandset.firstChild)
88+
this.commandset.removeChild(this.commandset.firstChild);
8189
}
8290

8391
// add all applicable commands
84-
function add_command(rule) {
85-
var command = document.createElement("command");
86-
command.setAttribute('id', rule.id+'-command');
87-
command.setAttribute('label', rule.name);
88-
command.setAttribute('oncommand', 'toggle_rule("'+rule.id+'")');
89-
commandset.appendChild(command);
90-
}
9192
for(var x in this.active)
92-
add_command(this.active[x]);
93+
this.add_command(this.active[x]);
9394
for(var x in this.moot)
94-
add_command(this.moot[x]);
95+
this.add_command(this.moot[x]);
9596
for(var x in this.inactive)
96-
add_command(this.inactive[x]);
97-
98-
99-
// add a menu item for a rule -- type is "active", "inactive", or "moot"
100-
function add_menuitem(rule, type) {
101-
// create the menuitem
102-
var item = document.createElement('menuitem');
103-
item.setAttribute('command', rule.id+'-command');
104-
item.setAttribute('class', type+'-item');
105-
106-
// set the icon
107-
var image = document.createElement('image');
108-
var image_src;
109-
if(type == 'active') image_src = 'tick.png';
110-
else if(type == 'inactive') image_src = 'cross.png';
111-
else if(type == 'moot') image_src = 'tick-moot.png';
112-
image.setAttribute('src', 'chrome://https-everywhere/skin/'+image_src);
113-
114-
// set the label
115-
var label = document.createElement('label');
116-
label.setAttribute('value', rule.name);
117-
118-
// put them in an hbox, and put the hbox in the menuitem
119-
var hbox = document.createElement('hbox');
120-
hbox.appendChild(image);
121-
hbox.appendChild(label);
122-
item.appendChild(hbox);
123-
124-
// all done
125-
menupopup.appendChild(item);
126-
}
97+
this.add_command(this.inactive[x]);
12798

12899
// add all the menu items
129100
for(var x in this.active)
130-
add_menuitem(this.active[x], 'active');
101+
this.add_menuitem(this.active[x], 'active');
131102
// rules that are active for some uris are not really moot
132103
for(var x in this.moot)
133104
if(!(x in this.active))
134-
add_menuitem(this.moot[x], 'moot');
105+
this.add_menuitem(this.moot[x], 'moot');
135106
for(var x in this.inactive)
136-
add_menuitem(this.inactive[x], 'inactive');
107+
this.add_menuitem(this.inactive[x], 'inactive');
137108

138109
// add other menu items
139-
menupopup.appendChild(document.createElement('menuseparator'));
110+
this.menupopup.appendChild(document.createElement('menuseparator'));
140111

141112
// preferences, about
142-
/*var preferences = document.createElement('menuitem');
143-
preferences.setAttribute('label', 'Preferences');
144-
preferences.setAttribute('command', 'https-everywhere-menuitem-preferences');
145-
menupopup.appendChild(preferences);*/
146113
var about = document.createElement('menuitem');
147114
about.setAttribute('label', 'About HTTPS Everywhere');
148115
about.setAttribute('command', 'https-everywhere-menuitem-about');
149-
menupopup.appendChild(about);
116+
this.menupopup.appendChild(about);
150117

151118
// separator
152-
menupopup.appendChild(document.createElement('menuseparator'));
119+
this.menupopup.appendChild(document.createElement('menuseparator'));
153120

154121
// donate
155122
var donate_eff = document.createElement('menuitem');
156123
donate_eff.setAttribute('label', 'Donate to EFF');
157124
donate_eff.setAttribute('command', 'https-everywhere-menuitem-donate-eff');
158-
menupopup.appendChild(donate_eff);
125+
this.menupopup.appendChild(donate_eff);
159126
var donate_tor = document.createElement('menuitem');
160127
donate_tor.setAttribute('label', 'Donate to Tor Project');
161128
donate_tor.setAttribute('command', 'https-everywhere-menuitem-donate-tor');
162-
menupopup.appendChild(donate_tor);
129+
this.menupopup.appendChild(donate_tor);
130+
131+
this.log(DBUG, "finished menu");
132+
133+
},
134+
135+
add_command: function(rule) {
136+
var command = this.document.createElement("command");
137+
command.setAttribute('id', rule.id+'-command');
138+
command.setAttribute('label', rule.name);
139+
command.setAttribute('oncommand', 'toggle_rule("'+rule.id+'")');
140+
this.commandset.appendChild(command);
141+
},
163142

164-
this.log(WARN, "finished menu");
143+
// add a menu item for a rule -- type is "active", "inactive", or "moot"
144+
add_menuitem: function(rule, type) {
145+
// create the menuitem
146+
var item = this.document.createElement('menuitem');
147+
item.setAttribute('command', rule.id+'-command');
148+
item.setAttribute('class', type+'-item');
149+
150+
// set the icon
151+
var image = this.document.createElement('image');
152+
var image_src;
153+
if(type == 'active') image_src = 'tick.png';
154+
else if(type == 'inactive') image_src = 'cross.png';
155+
else if(type == 'moot') image_src = 'tick-moot.png';
156+
image.setAttribute('src', 'chrome://https-everywhere/skin/'+image_src);
157+
158+
// set the label
159+
var label = this.document.createElement('label');
160+
label.setAttribute('value', rule.name);
165161

162+
// put them in an hbox, and put the hbox in the menuitem
163+
var hbox = this.document.createElement('hbox');
164+
hbox.appendChild(image);
165+
hbox.appendChild(label);
166+
item.appendChild(hbox);
167+
168+
// all done
169+
this.menupopup.appendChild(item);
166170
},
167171

168172
show_applicable: function() {

src/chrome/content/code/HTTPSRules.js

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,14 @@ RuleSet.prototype = {
8282
wouldMatch: function(hypothetical_uri, alist) {
8383
// return true if this ruleset would match the uri, assuming it were http
8484
// used for judging moot / inactive rulesets
85-
this.log(DBUG,"Would " +this.name + " match " +hypothetical_uri.spec +"? serial " + alist.serial);
85+
86+
// if the ruleset is already somewhere in this applicable list, we don't
87+
// care about hypothetical wouldMatch questios
88+
if (this.name in alist.all) return false;
89+
90+
this.log(DBUG,"Would " +this.name + " match " +hypothetical_uri.spec +
91+
"? serial " + alist.serial);
92+
8693
var uri = hypothetical_uri.clone();
8794
if (uri.scheme == "https") uri.scheme = "http";
8895
var urispec = uri.spec;
@@ -357,18 +364,18 @@ const HTTPSRules = {
357364
alist.inactive_rule(rs[i]);
358365
continue;
359366
}
360-
if (uri.scheme == "https" && alist) {
361-
// we didn't rewrite but the rule applies to this domain and the
362-
// requests are going over https
363-
if (rs[i].wouldMatch(uri, alist)) alist.moot_rule(rs[i]);
364-
continue;
365-
}
366367
newuri = rs[i].transformURI(uri);
367368
if (newuri) {
368369
// we rewrote the uri
369370
if (alist) alist.active_rule(rs[i]);
370371
return newuri;
371372
}
373+
if (uri.scheme == "https" && alist) {
374+
// we didn't rewrite but the rule applies to this domain and the
375+
// requests are going over https
376+
if (rs[i].wouldMatch(uri, alist)) alist.moot_rule(rs[i]);
377+
continue;
378+
}
372379
}
373380
return null;
374381
},

src/chrome/content/toolbar_button.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ function show_applicable_list() {
4646
var alist = HTTPSEverywhere.getExpando(domWin,"applicable_rules", null);
4747

4848
if (alist) {
49-
alist.log(WARN,"Success wherein domWin is " + domWin);
49+
//alist.log(WARN,"Success wherein domWin is " + domWin);
5050
alist.show_applicable();
5151
alist.populate_menu(document);
5252
} else {

src/components/https-everywhere.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ HTTPSEverywhere.prototype = {
319319
}
320320
var dw = domWin.top;
321321
this.log(DBUG, "Forcing new AL in getApplicableListForDOMWin in onLocationChange");
322-
var alist = new ApplicableList(this.log,dw.document);
322+
var alist = new ApplicableList(this.log,dw.document,dw);
323323
this.setExpando(dw,"applicable_rules",alist);
324324
return alist;
325325
},
@@ -335,7 +335,7 @@ HTTPSEverywhere.prototype = {
335335
this.log(DBUG,"get AL success in " + where);
336336
} else {
337337
this.log(DBUG, "Making new AL in getApplicableListForDOMWin in " + where);
338-
alist = new ApplicableList(this.log,dw.document);
338+
alist = new ApplicableList(this.log,dw.document,dw);
339339
this.setExpando(dw,"applicable_rules",alist);
340340
}
341341
return alist;
@@ -430,7 +430,7 @@ HTTPSEverywhere.prototype = {
430430
new_alist = old_alist;
431431
this.setExpando(domWin,"applicable_rules",new_alist);
432432
} else if (!new_alist) {
433-
new_alist = new ApplicableList(this.log, domWin.document);
433+
new_alist = new ApplicableList(this.log, domWin.document, domWin);
434434
this.setExpando(domWin,"applicable_rules",new_alist);
435435
}
436436
return new_alist;

0 commit comments

Comments
 (0)