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

Issue 13077005: Fix first-run page in Opera (Closed)

Created:
Oct. 16, 2013, 11:55 a.m. by Wladimir Palant
Modified:
Oct. 16, 2013, 1:44 p.m.
Reviewers:
Thomas Greiner
Visibility:
Public.

Description

Three individual commits here: (1) webNavigation is supported in current Opera versions, no special permissions required; (2) bogus data corruption warning due to callback being called too early; (3) asynchronous actions don`t wait for scripts to load

Patch Set 1 #

Patch Set 2 : Improved queuing of async actions #

Total comments: 4

Patch Set 3 : Fixed nits #

Unified diffs Side-by-side diffs Delta from patch set Stats (+44 lines, -15 lines) Patch
M .hgsubstate View 1 2 1 chunk +1 line, -1 line 0 comments Download
M lib/filesystem/io.js View 1 chunk +1 line, -1 line 0 comments Download
M lib/localstorage/io.js View 1 chunk +1 line, -1 line 0 comments Download
M lib/utils.js View 1 2 1 chunk +41 lines, -5 lines 0 comments Download
M metadata.opera View 1 chunk +0 lines, -7 lines 0 comments Download

Messages

Total messages: 4
Wladimir Palant
Oct. 16, 2013, 11:55 a.m. (2013-10-16 11:55:21 UTC) #1
Wladimir Palant
I've improved the approach for queuing async actions, it no longer relies on an explicit ...
Oct. 16, 2013, 1:14 p.m. (2013-10-16 13:14:52 UTC) #2
Thomas Greiner
Nice solution! LGTM with the nits addressed. http://codereview.adblockplus.org/13077005/diff/3001/lib/utils.js File lib/utils.js (right): http://codereview.adblockplus.org/13077005/diff/3001/lib/utils.js#newcode27 lib/utils.js:27: runAsync: function(callback, ...
Oct. 16, 2013, 1:36 p.m. (2013-10-16 13:36:46 UTC) #3
Wladimir Palant
Oct. 16, 2013, 1:44 p.m. (2013-10-16 13:44:41 UTC) #4
http://codereview.adblockplus.org/13077005/diff/3001/lib/utils.js
File lib/utils.js (right):

http://codereview.adblockplus.org/13077005/diff/3001/lib/utils.js#newcode27
lib/utils.js:27: runAsync: function(callback, thisPtr)
That's mainly because this isn't the canonical implementation of that function -
it's rather an emulation of a function available in Firefox. Still, I removed
the parameter and added a comment instead.

http://codereview.adblockplus.org/13077005/diff/3001/lib/utils.js#newcode33
lib/utils.js:33: runAsyncQueue = (document.readyState == "loading" ? [] : null)
On 2013/10/16 13:36:46, Thomas Greiner wrote:
> Nit: Semicolon at the end is missing.

Done.

Powered by Google App Engine
This is Rietveld