Issue 30018598: Issue 7281 - Remove Google+ links

Issue 30018598: Issue 7281 - Remove Google+ links (Closed)

Created:
Feb. 26, 2019, 12:07 p.m. by diegocarloslima

Modified:
March 18, 2019, 8:46 p.m.

Reviewers:
anton, jens

Visibility:
Public.

Patch Set 2 : Adjustments in regards to Anton's comments #

Created: Feb. 27, 2019, 5 p.m.

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+105 lines, -21 lines)			Patch
M	mobile/android/app/mobile.js	View		1 chunk	+1 line, -2 lines	0 comments	Download
R	mobile/android/app/src/main/res/raw/bookmarkdefaults_favicon_beta_community.png	View		Binary file		0 comments	Download
M	mobile/android/app/src/main/res/xml/preferences_vendor.xml	View		1 chunk	+3 lines, -3 lines	0 comments	Download
R	mobile/android/app/src/photon/res/drawable-hdpi/suggestedsites_adblockbrowserbetacommunity.png	View		Binary file		0 comments	Download
R	mobile/android/app/src/photon/res/drawable-xhdpi/suggestedsites_adblockbrowserbetacommunity.png	View		Binary file		0 comments	Download
R	mobile/android/app/src/photon/res/drawable-xxhdpi/suggestedsites_adblockbrowserbetacommunity.png	View		Binary file		0 comments	Download
A	mobile/android/base/java/org/adblockplus/browser/FeedbackPreference.java	View	1	1 chunk	+96 lines, -0 lines	0 comments	Download
M	mobile/android/base/moz.build	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	mobile/android/base/strings.xml.in	View		2 chunks	+2 lines, -7 lines	0 comments	Download
M	mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/StringHelper.java	View		1 chunk	+2 lines, -4 lines	0 comments	Download
M	mobile/locales/en-US/chrome/region.properties	View		1 chunk	+0 lines, -5 lines	0 comments	Download

Messages

Total messages: 7

Expand All Messages | Collapse All Messages

anton

https://codereview.adblockplus.org/30018598/diff/30018599/mobile/android/base/java/org/adblockplus/browser/FeedbackPreference.java File mobile/android/base/java/org/adblockplus/browser/FeedbackPreference.java (right): https://codereview.adblockplus.org/30018598/diff/30018599/mobile/android/base/java/org/adblockplus/browser/FeedbackPreference.java#newcode58 mobile/android/base/java/org/adblockplus/browser/FeedbackPreference.java:58: getContext().startActivity(intent); do we assume email client is always installed? ...

Feb. 26, 2019, 12:52 p.m. (2019-02-26 12:52:55 UTC) #3

diegocarloslima

Feb. 27, 2019, 4:37 p.m. (2019-02-27 16:37:57 UTC) #4

anton

On 2019/02/27 16:37:57, diegocarloslima wrote: > https://codereview.adblockplus.org/30018598/diff/30018599/mobile/android/base/java/org/adblockplus/browser/FeedbackPreference.java > File mobile/android/base/java/org/adblockplus/browser/FeedbackPreference.java > (right): > > https://codereview.adblockplus.org/30018598/diff/30018599/mobile/android/base/java/org/adblockplus/browser/FeedbackPreference.java#newcode58 ...

Feb. 27, 2019, 4:39 p.m. (2019-02-27 16:39:21 UTC) #5

anton

On 2019/02/27 16:39:21, anton wrote: > On 2019/02/27 16:37:57, diegocarloslima wrote: > > > https://codereview.adblockplus.org/30018598/diff/30018599/mobile/android/base/java/org/adblockplus/browser/FeedbackPreference.java ...

Feb. 28, 2019, 8:58 a.m. (2019-02-28 08:58:36 UTC) #6

jens

Feb. 28, 2019, 9:54 a.m. (2019-02-28 09:54:27 UTC) #7

On 2019/02/28 08:58:36, anton wrote:
> On 2019/02/27 16:39:21, anton wrote:
> > On 2019/02/27 16:37:57, diegocarloslima wrote:
> > >
> >
>
https://codereview.adblockplus.org/30018598/diff/30018599/mobile/android/base...
> > > File
> mobile/android/base/java/org/adblockplus/browser/FeedbackPreference.java
> > > (right):
> > > 
> > >
> >
>
https://codereview.adblockplus.org/30018598/diff/30018599/mobile/android/base...
> > >
mobile/android/base/java/org/adblockplus/browser/FeedbackPreference.java:58:
> > > getContext().startActivity(intent);
> > > On 2019/02/26 12:52:54, anton wrote:
> > > > do we assume email client is always installed? otherwise we should try {
}
> > > catch
> > > > {}
> > > 
> > > I don't think that an exception will ever occur, but I see your point,
will
> > add
> > > a try catch block to handle this gracefully.
> > 
> > Yeah, i don't think it will actually happen as most of users have email
> client,
> > but anyway.
> > 
> > > 
> > >
> >
>
https://codereview.adblockplus.org/30018598/diff/30018599/mobile/android/base...
> > >
mobile/android/base/java/org/adblockplus/browser/FeedbackPreference.java:87:
> > //
> > > TODO: Replace by ConfigurationCompat when the support lib is >= 26.1.0
> > > On 2019/02/26 12:52:54, anton wrote:
> > > > afaik somewhere in code style guide there is requirement to remove TODO:
> > from
> > > > code and create a ticket instead. How do you think?
> > > 
> > > Acknowledged.
> 
> LGTM

LGTM

Expand All Messages | Collapse All Messages