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

Issue 29339387: Issue 3890 - Fix "Downloading..." indication for subscriptions on the options page (Closed)

Created:
April 5, 2016, 5:26 p.m. by Sebastian Noack
Modified:
April 6, 2016, 11 p.m.
Reviewers:
Thomas Greiner, kzar
Visibility:
Public.

Description

Issue 3890 - Fix "Downloading..." indication for subscriptions on the options page

Patch Set 1 #

Total comments: 6

Patch Set 2 : Wrapped long line #

Total comments: 3

Patch Set 3 : Pass URL to Synchronizer.isExecuting() #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+42 lines, -45 lines) Patch
M background.js View 1 1 chunk +3 lines, -1 line 0 comments Download
M messageResponder.js View 1 2 2 chunks +8 lines, -5 lines 0 comments Download
M options.js View 1 2 4 chunks +31 lines, -39 lines 1 comment Download

Messages

Total messages: 7
Sebastian Noack
https://codereview.adblockplus.org/29339387/diff/29339388/background.js File background.js (left): https://codereview.adblockplus.org/29339387/diff/29339388/background.js#oldcode290 background.js:290: subscription.lastDownload = 0; This was wrong. The lastDownload property ...
April 5, 2016, 6:38 p.m. (2016-04-05 18:38:49 UTC) #1
kzar
IMHO we should get rid of the Object.observe stuff completely, that way we could skip ...
April 6, 2016, 8:53 a.m. (2016-04-06 08:53:38 UTC) #2
Sebastian Noack
On 2016/04/06 08:53:38, kzar wrote: > IMHO we should get rid of the Object.observe stuff ...
April 6, 2016, 9:13 a.m. (2016-04-06 09:13:01 UTC) #3
kzar
> Either way, just listening for the "downloading" event isn't sufficient. The subscription might already/still ...
April 6, 2016, 9:25 a.m. (2016-04-06 09:25:12 UTC) #4
Thomas Greiner
>> IMHO we should get rid of the Object.observe stuff completely > I agree, that ...
April 6, 2016, 2:19 p.m. (2016-04-06 14:19:43 UTC) #5
Sebastian Noack
https://codereview.adblockplus.org/29339387/diff/29339407/messageResponder.js File messageResponder.js (right): https://codereview.adblockplus.org/29339387/diff/29339407/messageResponder.js#newcode71 messageResponder.js:71: obj.isDownloading = Synchronizer.isExecuting(subscription); On 2016/04/06 14:19:43, Thomas Greiner wrote: ...
April 6, 2016, 5:22 p.m. (2016-04-06 17:22:23 UTC) #6
Thomas Greiner
April 6, 2016, 6:08 p.m. (2016-04-06 18:08:17 UTC) #7
LGTM

Powered by Google App Engine
This is Rietveld