Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1035)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 11 months ago by Wladimir Palant
Modified:
4 years, 10 months ago
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
4 years, 11 months ago (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 ...
4 years, 11 months ago (2014-12-17 19:42:50 UTC) #2
Wladimir Palant
4 years, 11 months ago (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 ...
4 years, 11 months ago (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 ...
4 years, 11 months ago (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 ...
4 years, 11 months ago (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 ...
4 years, 11 months ago (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: > > ...
4 years, 11 months ago (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, ...
4 years, 11 months ago (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 ...
4 years, 11 months ago (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 ...
4 years, 11 months ago (2014-12-19 17:45:37 UTC) #11
Thomas Greiner
LGTM
4 years, 11 months ago (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 ...
4 years, 11 months ago (2014-12-22 09:56:16 UTC) #13
Wladimir Palant
4 years, 10 months ago (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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5