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

Issue 4731979438227456: Issue 1663 - Emulate background page and implement proper message responder (Closed)

Created:
Dec. 17, 2014, 6:32 p.m. by Wladimir Palant
Modified:
Jan. 15, 2015, 1:03 p.m.
Visibility:
Public.

Description

I realized that the message responder will be the same for all platforms, so I`ve added this one. In order for this to work the background page had to be emulated properly however

Patch Set 1 #

Patch Set 2 : Fixed sender API #

Patch Set 3 : Properly convert Subscription objects as notification arguments #

Total comments: 10

Patch Set 4 : Rebased and addressed comments #

Patch Set 5 : Using PageMap for listeners #

Total comments: 2

Patch Set 6 : Using Services.vc #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+544 lines, -122 lines) Patch
M README.md View 1 chunk +8 lines, -2 lines 0 comments Download
A background.html View 1 chunk +29 lines, -0 lines 0 comments Download
A background.js View 1 2 3 4 5 1 chunk +172 lines, -0 lines 0 comments Download
A ext/background.js View 1 2 3 4 1 chunk +78 lines, -0 lines 0 comments Download
M ext/common.js View 1 2 3 4 2 chunks +57 lines, -3 lines 0 comments Download
M ext/content.js View 1 2 3 1 chunk +36 lines, -116 lines 0 comments Download
M firstRun.js View 1 2 3 1 chunk +4 lines, -1 line 0 comments Download
A messageResponder.js View 1 2 3 4 5 1 chunk +160 lines, -0 lines 2 comments Download

Messages

Total messages: 14
Wladimir Palant
Dec. 17, 2014, 6:32 p.m. (2014-12-17 18:32:46 UTC) #1
Wladimir Palant
I realized that there the ext API already defines a way to send a message ...
Dec. 17, 2014, 7:42 p.m. (2014-12-17 19:42:50 UTC) #2
Wladimir Palant
Dec. 17, 2014, 10:15 p.m. (2014-12-17 22:15:20 UTC) #3
Sebastian Noack
On 2014/12/17 19:42:50, Wladimir Palant wrote: > So I've adjusted the emulation here accordingly, the ...
Dec. 18, 2014, 12:58 p.m. (2014-12-18 12:58:39 UTC) #4
Thomas Greiner
On 2014/12/18 12:58:39, Sebastian Noack wrote: > I completely agree. The abstraction layer shouldn't send ...
Dec. 18, 2014, 1:27 p.m. (2014-12-18 13:27:49 UTC) #5
Thomas Greiner
http://codereview.adblockplus.org/4731979438227456/diff/5750085036015616/background.js File background.js (right): http://codereview.adblockplus.org/4731979438227456/diff/5750085036015616/background.js#newcode20 background.js:20: function getURLParameters(data) The function name is not reflecting what ...
Dec. 18, 2014, 5:32 p.m. (2014-12-18 17:32:05 UTC) #6
Wladimir Palant
On 2014/12/18 12:58:39, Sebastian Noack wrote: > Maybe it would be a better idea, to ...
Dec. 18, 2014, 10:04 p.m. (2014-12-18 22:04:44 UTC) #7
Sebastian Noack
On 2014/12/18 22:04:44, Wladimir Palant wrote: > On 2014/12/18 12:58:39, Sebastian Noack wrote: > > ...
Dec. 19, 2014, 1:26 p.m. (2014-12-19 13:26:19 UTC) #8
Wladimir Palant
Looks like I don't know the abstraction layer well-enough. The listeners work via PageMap now, ...
Dec. 19, 2014, 3:44 p.m. (2014-12-19 15:44:57 UTC) #9
Thomas Greiner
http://codereview.adblockplus.org/4731979438227456/diff/5759778777202688/messageResponder.js File messageResponder.js (right): http://codereview.adblockplus.org/4731979438227456/diff/5759778777202688/messageResponder.js#newcode89 messageResponder.js:89: parseInt(info.platformVersion, 10) < 6 || // beforeload breaks websites ...
Dec. 19, 2014, 4:58 p.m. (2014-12-19 16:58:23 UTC) #10
Wladimir Palant
http://codereview.adblockplus.org/4731979438227456/diff/5759778777202688/messageResponder.js File messageResponder.js (right): http://codereview.adblockplus.org/4731979438227456/diff/5759778777202688/messageResponder.js#newcode89 messageResponder.js:89: parseInt(info.platformVersion, 10) < 6 || // beforeload breaks websites ...
Dec. 19, 2014, 5:45 p.m. (2014-12-19 17:45:37 UTC) #11
Thomas Greiner
LGTM
Dec. 19, 2014, 5:59 p.m. (2014-12-19 17:59:49 UTC) #12
Sebastian Noack
http://codereview.adblockplus.org/4731979438227456/diff/5636470266134528/messageResponder.js File messageResponder.js (right): http://codereview.adblockplus.org/4731979438227456/diff/5636470266134528/messageResponder.js#newcode105 messageResponder.js:105: global.openOptions(); window.openOptions() is deprecated. Please use ext.openOptions(). And while ...
Dec. 22, 2014, 9:56 a.m. (2014-12-22 09:56:16 UTC) #13
Wladimir Palant
Jan. 15, 2015, 1:03 p.m. (2015-01-15 13:03:22 UTC) #14
http://codereview.adblockplus.org/4731979438227456/diff/5636470266134528/mess...
File messageResponder.js (right):

http://codereview.adblockplus.org/4731979438227456/diff/5636470266134528/mess...
messageResponder.js:105: global.openOptions();
On 2014/12/22 09:56:17, Sebastian Noack wrote:
> window.openOptions() is deprecated. Please use ext.openOptions(). And while
> implementing some parts of the abstraction layer for Firefox, you might want
to
> implement ext.openOptions() as well, and get rid of the switch above.

I filed #1813 and #1814 on that.

Powered by Google App Engine
This is Rietveld