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

Issue 5867629902299136: Fixed various issues in older Chrome versions (Closed)

Created:
Dec. 19, 2013, 10:24 a.m. by Wladimir Palant
Modified:
Dec. 19, 2013, 10:58 a.m.
Reviewers:
Sebastian Noack
Visibility:
Public.

Description

* There was an intermediate messaging API where messages were sent via chrome.extension.sendMessage/chrome.tabs.sendMessage and received with chrome.extension.onMessage * Accessing (not calling) chrome.extension.getBackgroundPage used to cause an exception * Messages received by content scripts got a fake sender tab with ID -1 * Pop-up should not assume always getting a response to its messages

Patch Set 1 #

Total comments: 2

Patch Set 2 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+36 lines, -18 lines) Patch
M chrome/background.js View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/common.js View 1 1 chunk +22 lines, -4 lines 0 comments Download
M popup.html View 1 chunk +5 lines, -5 lines 0 comments Download
M popup.js View 2 chunks +7 lines, -7 lines 0 comments Download

Messages

Total messages: 4
Wladimir Palant
Dec. 19, 2013, 10:24 a.m. (2013-12-19 10:24:21 UTC) #1
Sebastian Noack
http://codereview.adblockplus.org/5867629902299136/diff/5629499534213120/chrome/common.js File chrome/common.js (right): http://codereview.adblockplus.org/5867629902299136/diff/5629499534213120/chrome/common.js#newcode97 chrome/common.js:97: try I would prefer to wrap chrome.extension.getBackgroundPage() in a ...
Dec. 19, 2013, 10:38 a.m. (2013-12-19 10:38:58 UTC) #2
Wladimir Palant
http://codereview.adblockplus.org/5867629902299136/diff/5629499534213120/chrome/common.js File chrome/common.js (right): http://codereview.adblockplus.org/5867629902299136/diff/5629499534213120/chrome/common.js#newcode97 chrome/common.js:97: try True, that's a better solution - done.
Dec. 19, 2013, 10:47 a.m. (2013-12-19 10:47:17 UTC) #3
Sebastian Noack
Dec. 19, 2013, 10:52 a.m. (2013-12-19 10:52:09 UTC) #4
LGTM

Powered by Google App Engine
This is Rietveld