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

Issue 5767210236641280: Issue 2053 - Replace Adblock Plus logo on the first-run page (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years ago by Wladimir Palant
Modified:
5 years ago
Reviewers:
Thomas Greiner
Visibility:
Public.

Description

Issue 2053 - Replace Adblock Plus logo on the first-run page

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -3 lines) Patch
M skin/abp-icon-big.png View Binary file 0 comments Download
M skin/firstRun.css View 2 chunks +3 lines, -3 lines 3 comments Download

Messages

Total messages: 4
Wladimir Palant
5 years ago (2015-02-26 19:26:21 UTC) #1
Thomas Greiner
http://codereview.adblockplus.org/5767210236641280/diff/5629499534213120/skin/firstRun.css File skin/firstRun.css (right): http://codereview.adblockplus.org/5767210236641280/diff/5629499534213120/skin/firstRun.css#newcode158 skin/firstRun.css:158: background-image: url(abp-icon-big.png); For consistency's sake and to avoid duplication ...
5 years ago (2015-02-27 10:51:23 UTC) #2
Wladimir Palant
http://codereview.adblockplus.org/5767210236641280/diff/5629499534213120/skin/firstRun.css File skin/firstRun.css (right): http://codereview.adblockplus.org/5767210236641280/diff/5629499534213120/skin/firstRun.css#newcode158 skin/firstRun.css:158: background-image: url(abp-icon-big.png); On 2015/02/27 10:51:23, Thomas Greiner wrote: > ...
5 years ago (2015-02-27 16:17:25 UTC) #3
Thomas Greiner
5 years ago (2015-02-27 16:22:49 UTC) #4
LGTM

http://codereview.adblockplus.org/5767210236641280/diff/5629499534213120/skin...
File skin/firstRun.css (right):

http://codereview.adblockplus.org/5767210236641280/diff/5629499534213120/skin...
skin/firstRun.css:158: background-image: url(abp-icon-big.png);
On 2015/02/27 16:17:25, Wladimir Palant wrote:
> On 2015/02/27 10:51:23, Thomas Greiner wrote:
> > For consistency's sake and to avoid duplication I'd suggest renaming the
image
> > file to "abp-128.png".
> 
> I considered that already. However, avoiding duplication isn't as simple as it
> sounds - it would require moving abp-128.png to a new location in Chrome, so
> that it can be accessed under the same relative URL both in Firefox and Chrome
> (and this repository). And that would break consistency there.
> 
> So until we come up with a different structure for the adblockpluschrome
> repository, I'd rather keep the file name to make sure the dependency update
> isn't unnecessarily complicated.

Ok, I'm fine with addressing this later on.
Sign in to reply to this message.

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