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

Issue 4549521954570240: Added backwards compatibility for Chrome's old messaging API (Closed)

Created:
Nov. 15, 2013, 6:42 p.m. by Sebastian Noack
Modified:
Nov. 17, 2013, 1:25 p.m.
Visibility:
Public.

Description

Added backwards compatibility for Chrome's old messaging API

Patch Set 1 #

Total comments: 1

Patch Set 2 : Handled case when chrome.runtime isn't present #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -10 lines) Patch
M chrome/background.js View 2 chunks +3 lines, -1 line 0 comments Download
M chrome/common.js View 1 2 chunks +4 lines, -9 lines 0 comments Download

Messages

Total messages: 5
Sebastian Noack
Nov. 15, 2013, 6:43 p.m. (2013-11-15 18:43:57 UTC) #1
Felix Dahlke
LGTM
Nov. 16, 2013, 2:16 p.m. (2013-11-16 14:16:02 UTC) #2
Wladimir Palant
http://codereview.adblockplus.org/4549521954570240/diff/5629499534213120/chrome/common.js File chrome/common.js (right): http://codereview.adblockplus.org/4549521954570240/diff/5629499534213120/chrome/common.js#newcode76 chrome/common.js:76: sendMessage: chrome.runtime.sendMessage || chrome.extension.sendRequest, This will error out with ...
Nov. 17, 2013, 6:19 a.m. (2013-11-17 06:19:45 UTC) #3
Wladimir Palant
Actually, seeing that Yandex Browser moved on and is currently based on Chrome 28, I ...
Nov. 17, 2013, 6:23 a.m. (2013-11-17 06:23:06 UTC) #4
Sebastian Noack
Nov. 17, 2013, 1:25 p.m. (2013-11-17 13:25:56 UTC) #5
On 2013/11/17 06:23:06, Wladimir Palant wrote:
> Actually, seeing that Yandex Browser moved on and is currently based on Chrome
> 28, I wonder whether changing compat info to require Chrome 22 wouldn't be the
> better solution.

If this is everything that is required to stay compatible down to Chrome 18, I
would prefer to keep the backwards compatibility. Thanks to the new abstraction
layer, we only have to address compatibility at one place in our code.

Powered by Google App Engine
This is Rietveld