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

Issue 29341383: Issue 2606 - Firefox logo shown in background on empty Adblock Browser New Tab and Search pages (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 9 months ago by diegocarloslima
Modified:
3 years, 3 months ago
Reviewers:
Felix Dahlke, anton
CC:
René Jeschke
Visibility:
Public.

Description

Issue 2606 - Firefox logo shown in background on empty Adblock Browser New Tab and Search pages

Patch Set 1 #

Patch Set 2 : Adding missing drawable resources #

Patch Set 3 : Including drawable resources removal #

Patch Set 4 : Adding change comments #

Total comments: 2

Patch Set 5 : Adjustments in comments #

Messages

Total messages: 7
diegocarloslima
3 years, 9 months ago (2016-05-13 10:28:12 UTC) #1
diegocarloslima
3 years, 9 months ago (2016-05-18 09:11:42 UTC) #2
anton
On 2016/05/18 09:11:42, diegocarloslima wrote: LGTM
3 years, 4 months ago (2016-09-30 07:19:16 UTC) #3
Felix Dahlke
I wonder if we shouldn't just replace icon_home_empty_firefox.png with ours, that would be a less ...
3 years, 4 months ago (2016-09-30 07:28:55 UTC) #4
diegocarloslima
On 2016/09/30 07:28:55, Felix Dahlke wrote: > I wonder if we shouldn't just replace icon_home_empty_firefox.png ...
3 years, 4 months ago (2016-10-25 16:38:21 UTC) #5
Felix Dahlke
On 2016/10/25 16:38:21, diegocarloslima wrote: > On 2016/09/30 07:28:55, Felix Dahlke wrote: > > I ...
3 years, 4 months ago (2016-10-27 05:28:18 UTC) #6
Felix Dahlke
3 years, 3 months ago (2016-11-17 18:38:52 UTC) #7
LGTM with two nits. Feel free to address those, or not.

https://codereview.adblockplus.org/29341383/diff/29361535/mobile/android/base...
File mobile/android/base/home/PanelAuthLayout.java (right):

https://codereview.adblockplus.org/29341383/diff/29361535/mobile/android/base...
mobile/android/base/home/PanelAuthLayout.java:56: // Changed icon. See
https://issues.adblockplus.org/ticket/2606
Nit: Makes more sense to me if you'd move this one line down, i.e. after the
upstream comment. But not important.

https://codereview.adblockplus.org/29341383/diff/29361535/mobile/android/base...
File mobile/android/base/home/PanelLayout.java (right):

https://codereview.adblockplus.org/29341383/diff/29361535/mobile/android/base...
mobile/android/base/home/PanelLayout.java:463: // Changed icon. See
https://issues.adblockplus.org/ticket/2606
Nit: I guess it would suffice to have this comment once, above the block (i.e.
moving it one line up). But I don't mind either way, up to you AFAICT.
Sign in to reply to this message.

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