Issue 29331687: Issue 3223 - Make context menu e10s compatible in Firefox

Keyboard Shortcuts

	File
u :	up to issue
m :	publish + mail comments
M :	edit review message
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line
<Enter> :	respond to / edit current comment
d :	mark current comment as done

	Issue
u :	up to list of issues
m :	publish + mail comments
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue
# :	close issue

	Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :	save comment
<Esc> :	cancel edit

Issue 29331687: Issue 3223 - Make context menu e10s compatible in Firefox (Closed)

Created:
Dec. 1, 2015, 1:54 p.m. by Wladimir Palant

Modified:
Dec. 4, 2015, 12:05 p.m.

Reviewers:
Thomas Greiner, Erik, tschuster

Visibility:
Public.

Description

This is only the first part, the fallback for Thunderbird and older SeaMonkey versions still needs to be implemented.

Patch Set 2 : gContextMenuContentData exists in SeaMonkey Mail but is null #

Patch Set 3 : Don`t export getContextInfo function unnecessarily #

Total comments: 2

Created: Dec. 1, 2015, 2:37 p.m.

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+170 lines, -86 lines)			Patch
A	lib/child/contextMenu.js	View	1 2	1 chunk	+140 lines, -0 lines	0 comments	Download
M	lib/child/main.js	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	lib/ui.js	View	1	2 chunks	+29 lines, -86 lines	2 comments	Download

Messages

Total messages: 5

Expand All Messages | Collapse All Messages

Wladimir Palant

https://codereview.adblockplus.org/29331687/diff/29331688/lib/child/contextMenu.js File lib/child/contextMenu.js (right): https://codereview.adblockplus.org/29331687/diff/29331688/lib/child/contextMenu.js#newcode61 lib/child/contextMenu.js:61: // Look up data that we have for the ...

Dec. 1, 2015, 1:56 p.m. (2015-12-01 13:56:39 UTC) #2

Erik

https://codereview.adblockplus.org/29331687/diff/29331696/lib/ui.js File lib/ui.js (right): https://codereview.adblockplus.org/29331687/diff/29331696/lib/ui.js#newcode1667 lib/ui.js:1667: Policy.deleteNodes(id); I'm not quite sure what this piece is ...

Dec. 3, 2015, 11:21 p.m. (2015-12-03 23:21:58 UTC) #4

Wladimir Palant

Dec. 4, 2015, 12:05 p.m. (2015-12-04 12:05:10 UTC) #5

Message was sent while issue was closed.

https://codereview.adblockplus.org/29331687/diff/29331696/lib/ui.js
File lib/ui.js (right):

https://codereview.adblockplus.org/29331687/diff/29331696/lib/ui.js#newcode1667
lib/ui.js:1667: Policy.deleteNodes(id);
On 2015/12/03 23:21:57, Erik wrote:
> I'm not quite sure what this piece is for.

We are keeping a reference to the nodes in the content process - that reference
needs to be released, otherwise we are going to leak memory. Only exception is
the menu entry that was clicked, the corresponding ID has to be passed to the
filter assistant, it will then call Policy.deleteNodes() when done.

The point of the exercise: providing immediate feedback when the user adds a
filter, by hiding the element that it applies to. Yes, users need to see that
Adblock Plus did something - and not merely after a reload.

Expand All Messages | Collapse All Messages