jenkins-bot has submitted this change and it was merged.
Change subject: Fix possible NPE(s) when accessing parent fragment.
......................................................................
Fix possible NPE(s) when accessing parent fragment.
In our LeadImagesHandler and BottomContentHandler, we use the
getFragment() function which returns the "internal" fragment object that
is currently hosted by the PageViewFragment. The problem is that, when a
PageViewFragment is detached, it sets its instance of the internal object
to null. So, if the Lead or Bottom components try to access the internal
fragment using getFragment() from the result of an AsyncTask after the
parent fragment is detached, they will get a null pointer.
This patch refactors things so that the internal object itself is passed
to the Lead and Bottom components, so that they will never receive a null
reference to it.
Change-Id: Ie78c76e12128a89218bcea8d65fbfeed01278a63
---
M wikipedia/src/main/java/org/wikipedia/page/PageViewFragmentInternal.java
M
wikipedia/src/main/java/org/wikipedia/page/bottomcontent/BottomContentHandler.java
M wikipedia/src/main/java/org/wikipedia/page/leadimages/LeadImagesHandler.java
3 files changed, 39 insertions(+), 58 deletions(-)
Approvals:
BearND: Looks good to me, approved
jenkins-bot: Verified
diff --git
a/wikipedia/src/main/java/org/wikipedia/page/PageViewFragmentInternal.java
b/wikipedia/src/main/java/org/wikipedia/page/PageViewFragmentInternal.java
index eb3efc8..9f7fd44 100644
--- a/wikipedia/src/main/java/org/wikipedia/page/PageViewFragmentInternal.java
+++ b/wikipedia/src/main/java/org/wikipedia/page/PageViewFragmentInternal.java
@@ -416,9 +416,9 @@
new SearchBarHideHandler(webView, getActivity().getToolbarView());
imagesContainer = (ViewGroup)
parentFragment.getView().findViewById(R.id.page_images_container);
- leadImagesHandler = new LeadImagesHandler(parentFragment, bridge,
webView, imagesContainer);
+ leadImagesHandler = new LeadImagesHandler(getActivity(), this, bridge,
webView, imagesContainer);
- bottomContentHandler = new BottomContentHandler(parentFragment, bridge,
+ bottomContentHandler = new BottomContentHandler(this, bridge,
webView, linkHandler,
(ViewGroup)
parentFragment.getView().findViewById(R.id.bottom_content_container),
title);
diff --git
a/wikipedia/src/main/java/org/wikipedia/page/bottomcontent/BottomContentHandler.java
b/wikipedia/src/main/java/org/wikipedia/page/bottomcontent/BottomContentHandler.java
index 97076b5..4d9f00f 100644
---
a/wikipedia/src/main/java/org/wikipedia/page/bottomcontent/BottomContentHandler.java
+++
b/wikipedia/src/main/java/org/wikipedia/page/bottomcontent/BottomContentHandler.java
@@ -31,7 +31,7 @@
import org.wikipedia.page.LinkMovementMethodExt;
import org.wikipedia.page.Page;
import org.wikipedia.page.PageActivity;
-import org.wikipedia.page.PageViewFragment;
+import org.wikipedia.page.PageViewFragmentInternal;
import org.wikipedia.page.SuggestionsTask;
import org.wikipedia.search.FullSearchArticlesTask;
import org.wikipedia.views.ObservableWebView;
@@ -44,7 +44,7 @@
public class BottomContentHandler implements
ObservableWebView.OnScrollChangeListener,
ObservableWebView.OnContentHeightChangedListener {
private static final String TAG = "BottomContentHandler";
- private final PageViewFragment parentFragment;
+ private final PageViewFragmentInternal parentFragment;
private final CommunicationBridge bridge;
private final WebView webView;
private final LinkHandler linkHandler;
@@ -63,7 +63,7 @@
private SuggestedPagesFunnel funnel;
private FullSearchArticlesTask.FullSearchResults readMoreItems;
- public BottomContentHandler(PageViewFragment parentFragment,
CommunicationBridge bridge,
+ public BottomContentHandler(PageViewFragmentInternal parentFragment,
CommunicationBridge bridge,
ObservableWebView webview, LinkHandler
linkHandler,
ViewGroup hidingView, PageTitle pageTitle) {
this.parentFragment = parentFragment;
@@ -71,9 +71,9 @@
this.webView = webview;
this.linkHandler = linkHandler;
this.pageTitle = pageTitle;
- activity = parentFragment.getFragment().getActivity();
+ activity = parentFragment.getActivity();
app = (WikipediaApp) activity.getApplicationContext();
- displayDensity =
parentFragment.getResources().getDisplayMetrics().density;
+ displayDensity = activity.getResources().getDisplayMetrics().density;
bottomContentContainer = hidingView;
webview.addOnScrollChangeListener(this);
@@ -139,7 +139,7 @@
funnel = new SuggestedPagesFunnel(app, pageTitle.getSite());
// preload the display density, since it will be used in a lot of
places
- displayDensity =
parentFragment.getResources().getDisplayMetrics().density;
+ displayDensity = activity.getResources().getDisplayMetrics().density;
// hide ourselves by default
hide();
}
@@ -193,7 +193,7 @@
public void beginLayout() {
setupAttribution();
- if (parentFragment.getFragment().getPage().couldHaveReadMoreSection())
{
+ if (parentFragment.getPage().couldHaveReadMoreSection()) {
requestReadMoreItems(activity.getLayoutInflater());
} else {
bottomContentContainer.findViewById(R.id.read_more_container).setVisibility(View.GONE);
@@ -222,7 +222,7 @@
ListAdapter adapter = readMoreList.getAdapter();
if (adapter != null && adapter.getCount() > 0) {
ViewGroup.LayoutParams params = readMoreList.getLayoutParams();
- final int itemHeight =
(int)parentFragment.getResources().getDimension(R.dimen.defaultListItemSize);
+ final int itemHeight =
(int)activity.getResources().getDimension(R.dimen.defaultListItemSize);
params.height = adapter.getCount() * itemHeight
+ (readMoreList.getDividerHeight() *
(adapter.getCount() - 1));
readMoreList.setLayoutParams(params);
@@ -245,14 +245,14 @@
}
private void setupAttribution() {
- Page page = parentFragment.getFragment().getPage();
+ Page page = parentFragment.getPage();
String lastUpdatedHtml = "<a href=\"" +
page.getTitle().getUriForAction("history")
- + "\">" + parentFragment.getString(R.string.last_updated_text,
+ + "\">" + activity.getString(R.string.last_updated_text,
Utils.formatDateRelative(page.getPageProperties().getLastModified())
+ "</a>");
pageLastUpdatedText.setText(Html.fromHtml(lastUpdatedHtml));
pageLastUpdatedText.setMovementMethod(new
LinkMovementMethodExt(linkHandler));
-
pageLicenseText.setText(Html.fromHtml(parentFragment.getString(R.string.content_license_html)));
+
pageLicenseText.setText(Html.fromHtml(activity.getString(R.string.content_license_html)));
pageLicenseText.setMovementMethod(new
LinkMovementMethodExt(linkHandler));
}
diff --git
a/wikipedia/src/main/java/org/wikipedia/page/leadimages/LeadImagesHandler.java
b/wikipedia/src/main/java/org/wikipedia/page/leadimages/LeadImagesHandler.java
index def891c..81fdd8c 100644
---
a/wikipedia/src/main/java/org/wikipedia/page/leadimages/LeadImagesHandler.java
+++
b/wikipedia/src/main/java/org/wikipedia/page/leadimages/LeadImagesHandler.java
@@ -1,5 +1,6 @@
package org.wikipedia.page.leadimages;
+import android.content.Context;
import android.graphics.Bitmap;
import android.graphics.Color;
import android.graphics.Point;
@@ -29,9 +30,6 @@
import org.wikipedia.ViewAnimations;
import org.wikipedia.WikipediaApp;
import org.wikipedia.bridge.CommunicationBridge;
-import org.wikipedia.page.Page;
-import org.wikipedia.page.PageProperties;
-import org.wikipedia.page.PageViewFragment;
import org.wikipedia.page.PageViewFragmentInternal;
import org.wikipedia.views.ObservableWebView;
import org.wikipedia.wikidata.WikidataCache;
@@ -39,7 +37,8 @@
import java.util.Map;
public class LeadImagesHandler implements
ObservableWebView.OnScrollChangeListener, ImageViewWithFace.OnImageLoadListener
{
- private final PageViewFragment parentFragment;
+ private final Context context;
+ private final PageViewFragmentInternal parentFragment;
private final CommunicationBridge bridge;
private final WebView webView;
@@ -106,13 +105,15 @@
void onLayoutComplete();
}
- public LeadImagesHandler(final PageViewFragment parentFragment,
CommunicationBridge bridge,
- ObservableWebView webview, ViewGroup hidingView) {
+ public LeadImagesHandler(final Context context, final
PageViewFragmentInternal parentFragment,
+ CommunicationBridge bridge, ObservableWebView
webview,
+ ViewGroup hidingView) {
+ this.context = context;
this.parentFragment = parentFragment;
this.imageContainer = hidingView;
this.bridge = bridge;
this.webView = webview;
- displayDensity =
parentFragment.getResources().getDisplayMetrics().density;
+ displayDensity = context.getResources().getDisplayMetrics().density;
imagePlaceholder =
(ImageView)imageContainer.findViewById(R.id.page_image_placeholder);
image1 =
(ImageViewWithFace)imageContainer.findViewById(R.id.page_image_1);
@@ -122,7 +123,7 @@
webview.addOnScrollChangeListener(this);
// preload the display density, since it will be used in a lot of
places
- displayDensity =
parentFragment.getResources().getDisplayMetrics().density;
+ displayDensity = context.getResources().getDisplayMetrics().density;
// get the screen height, using correct methods for different API
versions
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.HONEYCOMB_MR2) {
@@ -137,15 +138,13 @@
image1.setOnClickListener(new View.OnClickListener() {
@Override
public void onClick(View v) {
- String imageName =
parentFragment.getFragment().getPage().getPageProperties()
- .getLeadImageName();
+ String imageName =
parentFragment.getPage().getPageProperties().getLeadImageName();
if (imageName == null) {
return;
}
PageTitle imageTitle = new PageTitle("File:" + imageName,
-
parentFragment.getFragment().getTitle()
- .getSite());
- parentFragment.getFragment().showImageGallery(imageTitle);
+
parentFragment.getTitle().getSite());
+ parentFragment.showImageGallery(imageTitle);
}
});
@@ -276,7 +275,7 @@
* @param listener Listener that will receive an event when the layout is
completed.
*/
public void beginLayout(OnLeadImageLayoutListener listener) {
- String thumbUrl = getLeadImageUrl();
+ String thumbUrl =
parentFragment.getPage().getPageProperties().getLeadImageUrl();
if (!WikipediaApp.getInstance().showImages() || displayHeight <
MIN_SCREEN_HEIGHT_DP
|| WikipediaApp.getInstance().getReleaseType() ==
WikipediaApp.RELEASE_PROD) {
@@ -288,30 +287,13 @@
}
// set the page title text, and honor any HTML formatting in the title
-
pageTitleText.setText(Html.fromHtml(parentFragment.getFragment().getPage().getDisplayTitle()));
+
pageTitleText.setText(Html.fromHtml(parentFragment.getPage().getDisplayTitle()));
// hide the description text...
pageDescriptionText.setVisibility(View.INVISIBLE);
// kick off the (asynchronous) laying out of the page title text
-
layoutPageTitle((int)(parentFragment.getResources().getDimension(R.dimen.titleTextSize)
+
layoutPageTitle((int)(context.getResources().getDimension(R.dimen.titleTextSize)
/ displayDensity), listener);
- }
-
- // NPE, you shall not pass: https://phabricator.wikimedia.org/T78501
- private String getLeadImageUrl() {
- final PageViewFragmentInternal internalFragment =
parentFragment.getFragment();
- if (internalFragment == null) {
- return null;
- }
- final Page page = internalFragment.getPage();
- if (page == null) {
- return null;
- }
- final PageProperties pageProperties = page.getPageProperties();
- if (pageProperties == null) {
- return null;
- }
- return pageProperties.getLeadImageUrl();
}
/**
@@ -375,7 +357,7 @@
if (!parentFragment.isAdded()) {
return;
}
- boolean isMainPage =
parentFragment.getFragment().getPage().getPageProperties().isMainPage();
+ boolean isMainPage =
parentFragment.getPage().getPageProperties().isMainPage();
int titleContainerHeight;
int titleBottomPadding = 0;
@@ -398,17 +380,16 @@
image1.setVisibility(View.GONE);
imagePlaceholder.setVisibility(View.GONE);
// set the color of the title
- pageTitleText.setTextColor(parentFragment
- .getResources()
-
.getColor(Utils.getThemedAttributeId(parentFragment.getActivity(),
R.attr.lead_disabled_text_color)));
+ pageTitleText.setTextColor(context.getResources()
+
.getColor(Utils.getThemedAttributeId(parentFragment.getActivity(),
+
R.attr.lead_disabled_text_color)));
// remove bottom padding from the description
pageDescriptionText.setPadding(pageDescriptionText.getPaddingLeft(),
pageDescriptionText.getPaddingTop(),
pageDescriptionText.getPaddingRight(), 0);
// and give it no drop shadow
pageTitleText.setShadowLayer(0, 0, 0, 0);
// do the same for the description...
- pageDescriptionText.setTextColor(parentFragment
- .getResources()
+ pageDescriptionText.setTextColor(context.getResources()
.getColor(Utils.getThemedAttributeId(parentFragment.getActivity(),
R.attr.lead_disabled_text_color)));
pageDescriptionText.setShadowLayer(0, 0, 0, 0);
// remove any background from the title container
@@ -427,14 +408,14 @@
imagePlaceholder.setVisibility(View.VISIBLE);
// set the color of the title
-
pageTitleText.setTextColor(parentFragment.getResources().getColor(R.color.lead_text_color));
+
pageTitleText.setTextColor(context.getResources().getColor(R.color.lead_text_color));
final int bottomPaddingNominal = 16;
titleBottomPadding = (int)(bottomPaddingNominal * displayDensity);
// and give it a nice drop shadow!
- pageTitleText.setShadowLayer(2, 1, 1,
parentFragment.getResources().getColor(R.color.lead_text_shadow));
+ pageTitleText.setShadowLayer(2, 1, 1,
context.getResources().getColor(R.color.lead_text_shadow));
// do the same for the description...
-
pageDescriptionText.setTextColor(parentFragment.getResources().getColor(R.color.lead_text_color));
- pageDescriptionText.setShadowLayer(2, 1, 1,
parentFragment.getResources().getColor(R.color.lead_text_shadow));
+
pageDescriptionText.setTextColor(context.getResources().getColor(R.color.lead_text_color));
+ pageDescriptionText.setShadowLayer(2, 1, 1,
context.getResources().getColor(R.color.lead_text_shadow));
// set the title container background to be a gradient
pageTitleContainer.setBackgroundResource(R.drawable.lead_title_gradient);
// set the correct padding on the container
@@ -466,7 +447,7 @@
bridge.sendMessage("setPaddingTop", payload);
// and start fetching the lead image, if we have one
- String thumbUrl = getLeadImageUrl();
+ String thumbUrl =
parentFragment.getPage().getPageProperties().getLeadImageUrl();
if (!isMainPage && thumbUrl != null && leadImagesEnabled) {
thumbUrl = WikipediaApp.getInstance().getNetworkProtocol() + ":" +
thumbUrl;
Picasso.with(parentFragment.getActivity())
@@ -493,7 +474,7 @@
* of loading the WebView contents.
*/
private void fetchWikiDataDescription() {
- final PageTitle pageTitle =
parentFragment.getFragment().getPage().getTitle();
+ final PageTitle pageTitle = parentFragment.getPage().getTitle();
if (pageTitle != null) {
final String language = pageTitle.getSite().getLanguage();
--
To view, visit https://gerrit.wikimedia.org/r/181068
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie78c76e12128a89218bcea8d65fbfeed01278a63
Gerrit-PatchSet: 2
Gerrit-Project: apps/android/wikipedia
Gerrit-Branch: master
Gerrit-Owner: Dbrant <dbr...@wikimedia.org>
Gerrit-Reviewer: BearND <bsitzm...@wikimedia.org>
Gerrit-Reviewer: Brion VIBBER <br...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
