jenkins-bot has submitted this change and it was merged.
Change subject: Change way to get title from links
......................................................................
Change way to get title from links
For the mobileview/MW API case we can simply use the title
attribute instead of decoding the href attribute.
For the Parsoid/MCS case newer revisions of pages and once we get
all pages re-rendered after Icae4b83278791ed09284bb73c3ba80363cac9202
gets deployed will also have a title attribute.
For older pages the title attribute doesn't exists.
But we can derive it in JavaScript code from the href attribute by
removing a starting "/wiki/" or "./" and running decodeURIComponent on it.
That way the JS code always returns a title property, which the Java
side can just take as is.
The onUrlClick() method gets a new parameter titleString.
Instead of using getSite().titleForInternalLink(href) we now create the
PageTitle directly using titleString.
In the case of the gallery we don't get the link data from native code
instead of JS. The parsing of the HTML there is done using Html.fromHtml()
which doesn't preserve the title attribute of the link. In that one case
the Java code will derive the title from the href attribute like it did before.
This case is OK since the gallery data is not coming from Parsoid.
Introducing one factory method to create a PageTitle with a potential
fragment passed in separately. There was already a constructor with three
String parameters.
Related: Icae4b83278791ed09284bb73c3ba80363cac9202
Bug: T136223
Change-Id: Iedbe1ae611f4d433d4c6bcac3cce3935333753e0
---
M app/src/main/assets/bundle.js
M app/src/main/assets/preview.js
M app/src/main/java/org/wikipedia/editing/EditPreviewFragment.java
M app/src/main/java/org/wikipedia/editing/EditSectionActivity.java
M app/src/main/java/org/wikipedia/page/LinkHandler.java
M app/src/main/java/org/wikipedia/page/LinkMovementMethodExt.java
M app/src/main/java/org/wikipedia/page/PageFragment.java
M app/src/main/java/org/wikipedia/page/PageTitle.java
M app/src/main/java/org/wikipedia/page/gallery/GalleryActivity.java
M app/src/main/java/org/wikipedia/util/UriUtil.java
M app/src/main/java/org/wikipedia/wiktionary/WiktionaryDialog.java
A app/src/test/java/org/wikipedia/util/UriUtilTest.java
M www/js/actions.js
13 files changed, 155 insertions(+), 18 deletions(-)
Approvals:
jenkins-bot: Verified
Niedzielski: Looks good to me, approved
diff --git a/app/src/main/assets/bundle.js b/app/src/main/assets/bundle.js
index 2a21e83..f757c6d 100644
--- a/app/src/main/assets/bundle.js
+++ b/app/src/main/assets/bundle.js
@@ -39,6 +39,19 @@
}
}
+/**
+ * Either gets the title from the title attribute (for mobileview case and
newer MCS pages) or,
+ * if that doesn't not exists try to derive it from the href attribute value.
+ * In the latter case it also unescapes HTML entities to get the correct title
string.
+ */
+function getTitle( sourceNode, href ) {
+ if (sourceNode.hasAttribute( "title" )) {
+ return sourceNode.getAttribute( "title" );
+ } else {
+ return href.replace(/^\/wiki\//, '').replace(/^\.\//,
'').replace(/#.*$/, '');
+ }
+}
+
document.onclick = function() {
var sourceNode = null;
var curNode = event.target;
@@ -69,7 +82,7 @@
} else if (sourceNode.classList.contains( 'image' )) {
bridge.sendMessage( 'imageClicked', { "href": href } );
} else {
- bridge.sendMessage( 'linkClicked', { "href": href } );
+ bridge.sendMessage( 'linkClicked', { "href": href, "title":
getTitle(sourceNode, href) } );
}
event.preventDefault();
}
diff --git a/app/src/main/assets/preview.js b/app/src/main/assets/preview.js
index a8c2235..49c7fb1 100644
--- a/app/src/main/assets/preview.js
+++ b/app/src/main/assets/preview.js
@@ -39,6 +39,19 @@
}
}
+/**
+ * Either gets the title from the title attribute (for mobileview case and
newer MCS pages) or,
+ * if that doesn't not exists try to derive it from the href attribute value.
+ * In the latter case it also unescapes HTML entities to get the correct title
string.
+ */
+function getTitle( sourceNode, href ) {
+ if (sourceNode.hasAttribute( "title" )) {
+ return sourceNode.getAttribute( "title" );
+ } else {
+ return href.replace(/^\/wiki\//, '').replace(/^\.\//,
'').replace(/#.*$/, '');
+ }
+}
+
document.onclick = function() {
var sourceNode = null;
var curNode = event.target;
@@ -69,7 +82,7 @@
} else if (sourceNode.classList.contains( 'image' )) {
bridge.sendMessage( 'imageClicked', { "href": href } );
} else {
- bridge.sendMessage( 'linkClicked', { "href": href } );
+ bridge.sendMessage( 'linkClicked', { "href": href, "title":
getTitle(sourceNode, href) } );
}
event.preventDefault();
}
diff --git a/app/src/main/java/org/wikipedia/editing/EditPreviewFragment.java
b/app/src/main/java/org/wikipedia/editing/EditPreviewFragment.java
index 617959c..d01cfe0 100644
--- a/app/src/main/java/org/wikipedia/editing/EditPreviewFragment.java
+++ b/app/src/main/java/org/wikipedia/editing/EditPreviewFragment.java
@@ -6,6 +6,8 @@
import android.content.res.Configuration;
import android.content.res.Resources;
import android.os.Bundle;
+import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
import android.support.v4.app.Fragment;
import android.support.v7.app.AlertDialog;
import android.util.DisplayMetrics;
@@ -27,6 +29,7 @@
import org.wikipedia.page.PageActivity;
import org.wikipedia.page.PageTitle;
import org.wikipedia.util.L10nUtil;
+import org.wikipedia.util.UriUtil;
import org.wikipedia.views.ObservableWebView;
import org.wikipedia.bridge.CommunicationBridge;
import org.wikipedia.editing.summaries.EditSummaryTag;
@@ -190,17 +193,17 @@
}
@Override
- public void onUrlClick(final String href) {
+ public void onUrlClick(@NonNull final String href, @Nullable
final String titleString) {
// Check if this is an internal link, and if it is then
open it internally
if (href.startsWith("/wiki/")) {
- PageTitle title = getSite().titleForInternalLink(href);
+ PageTitle title =
PageTitle.withSeparateFragment(titleString, UriUtil.getFragment(href),
getSite());
onInternalLinkClicked(title);
} else {
//Show dialogue asking user to confirm they want to
leave
showLeavingEditDialogue(new Runnable() {
@Override
public void run() {
- openExternalLink(href);
+ openExternalLink(href, titleString);
}
});
}
@@ -244,9 +247,10 @@
* of LinkHandler to do the heavy lifting. You can't call this
method from inside a
* Runnable or an AlertDialog, so we put it in here instead.
* @param href The href of the external link to be opened.
+ * @param titleString the title of the page to be openend as a
string
*/
- private void openExternalLink(String href) {
- super.onUrlClick(href);
+ private void openExternalLink(String href, String titleString)
{
+ super.onUrlClick(href, titleString);
}
@Override
diff --git a/app/src/main/java/org/wikipedia/editing/EditSectionActivity.java
b/app/src/main/java/org/wikipedia/editing/EditSectionActivity.java
index 1cf3b8d..44b7e3e 100644
--- a/app/src/main/java/org/wikipedia/editing/EditSectionActivity.java
+++ b/app/src/main/java/org/wikipedia/editing/EditSectionActivity.java
@@ -228,7 +228,7 @@
editLicenseText.setMovementMethod(new LinkMovementMethodExt(new
LinkMovementMethodExt.UrlHandler() {
@Override
- public void onUrlClick(String url) {
+ public void onUrlClick(@NonNull String url, @Nullable String
notUsed) {
if (url.equals("https://#login";)) {
funnel.logLoginAttempt();
Intent loginIntent =
LoginActivity.newIntent(EditSectionActivity.this,
diff --git a/app/src/main/java/org/wikipedia/page/LinkHandler.java
b/app/src/main/java/org/wikipedia/page/LinkHandler.java
index 553ed39..44c806f 100644
--- a/app/src/main/java/org/wikipedia/page/LinkHandler.java
+++ b/app/src/main/java/org/wikipedia/page/LinkHandler.java
@@ -2,12 +2,15 @@
import android.content.Context;
import android.net.Uri;
+import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
import android.util.Log;
import org.json.JSONException;
import org.json.JSONObject;
import org.wikipedia.Site;
import org.wikipedia.bridge.CommunicationBridge;
+import org.wikipedia.util.UriUtil;
import static org.wikipedia.util.UriUtil.decodeURL;
import static org.wikipedia.util.UriUtil.handleExternalLink;
@@ -37,7 +40,7 @@
public void onMessage(String messageType, JSONObject messagePayload) {
try {
String href = decodeURL(messagePayload.getString("href"));
- onUrlClick(href);
+ onUrlClick(href, messagePayload.getString("title"));
} catch (IllegalArgumentException e) {
// The URL is malformed and URL decoder can't understand it. Just
do nothing.
Log.d("Wikipedia", "A malformed URL was tapped.");
@@ -47,14 +50,14 @@
}
@Override
- public void onUrlClick(String href) {
+ public void onUrlClick(@NonNull String href, @Nullable String titleString)
{
if (href.startsWith("//")) {
// That's a protocol specific link! Make it https!
href = "https:" + href;
}
Log.d("Wikipedia", "Link clicked was " + href);
if (href.startsWith("/wiki/")) {
- PageTitle title = getSite().titleForInternalLink(href);
+ PageTitle title = PageTitle.withSeparateFragment(titleString,
UriUtil.getFragment(href), getSite());
onInternalLinkClicked(title);
} else if (href.startsWith("#")) {
onPageLinkClicked(href.substring(1));
diff --git a/app/src/main/java/org/wikipedia/page/LinkMovementMethodExt.java
b/app/src/main/java/org/wikipedia/page/LinkMovementMethodExt.java
index af59dc5..8b451c1 100644
--- a/app/src/main/java/org/wikipedia/page/LinkMovementMethodExt.java
+++ b/app/src/main/java/org/wikipedia/page/LinkMovementMethodExt.java
@@ -1,6 +1,9 @@
package org.wikipedia.page;
+import org.wikipedia.util.UriUtil;
+
import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
import android.text.Layout;
import android.text.Spannable;
import android.text.method.LinkMovementMethod;
@@ -32,7 +35,7 @@
final URLSpan[] links = buffer.getSpans(off, off, URLSpan.class);
if (links.length != 0) {
String url = decodeURL(links[0].getURL());
- handler.onUrlClick(url);
+ handler.onUrlClick(url, UriUtil.getTitleFromUrl(url));
return true;
}
}
@@ -41,6 +44,6 @@
public interface UrlHandler {
- void onUrlClick(String url);
+ void onUrlClick(@NonNull String url, @Nullable String titleString);
}
}
\ No newline at end of file
diff --git a/app/src/main/java/org/wikipedia/page/PageFragment.java
b/app/src/main/java/org/wikipedia/page/PageFragment.java
index c56c2e8..83ea1b7 100755
--- a/app/src/main/java/org/wikipedia/page/PageFragment.java
+++ b/app/src/main/java/org/wikipedia/page/PageFragment.java
@@ -1051,7 +1051,7 @@
GalleryActivity.showGallery(getActivity(),
model.getTitleOriginal(),
filename, site,
GalleryFunnel.SOURCE_NON_LEAD_IMAGE);
} else {
- linkHandler.onUrlClick(href);
+ linkHandler.onUrlClick(href,
messagePayload.getString("title"));
}
} catch (JSONException e) {
L.logRemoteErrorIfProd(e);
diff --git a/app/src/main/java/org/wikipedia/page/PageTitle.java
b/app/src/main/java/org/wikipedia/page/PageTitle.java
index 029a830..b93d5a0 100644
--- a/app/src/main/java/org/wikipedia/page/PageTitle.java
+++ b/app/src/main/java/org/wikipedia/page/PageTitle.java
@@ -60,6 +60,25 @@
@Nullable private String description;
@Nullable private final PageProperties properties;
+ /**
+ * Creates a new PageTitle object.
+ * Use this if you want to pass in a fragment portion separately from the
title.
+ *
+ * @param prefixedText title of the page with optional namespace prefix
+ * @param fragment optional fragment portion
+ * @param site the wiki site the page belongs to
+ * @return a new PageTitle object matching the given input parameters
+ */
+ public static PageTitle withSeparateFragment(@Nullable String
prefixedText, @Nullable String fragment, @NonNull Site site) {
+ if (TextUtils.isEmpty(fragment)) {
+ return new PageTitle(prefixedText, site, null, (PageProperties)
null);
+ } else {
+ // TODO: this class needs some refactoring to allow passing in a
fragment
+ // without having to do string manipulations.
+ return new PageTitle(prefixedText + "#" + fragment, site, null,
(PageProperties) null);
+ }
+ }
+
public PageTitle(@Nullable final String namespace, @NonNull String text,
@Nullable String fragment, @Nullable String thumbUrl, @NonNull Site site) {
this.namespace = namespace;
this.text = text;
diff --git a/app/src/main/java/org/wikipedia/page/gallery/GalleryActivity.java
b/app/src/main/java/org/wikipedia/page/gallery/GalleryActivity.java
index 4ad131b..9b52e54 100644
--- a/app/src/main/java/org/wikipedia/page/gallery/GalleryActivity.java
+++ b/app/src/main/java/org/wikipedia/page/gallery/GalleryActivity.java
@@ -368,7 +368,7 @@
private LinkMovementMethodExt linkMovementMethod =
new LinkMovementMethodExt(new LinkMovementMethodExt.UrlHandler() {
@Override
- public void onUrlClick(String url) {
+ public void onUrlClick(@NonNull String url, @Nullable String notUsed) {
L.v("Link clicked was " + url);
url = resolveProtocolRelativeUrl(url);
Site appSite = app.getSite();
diff --git a/app/src/main/java/org/wikipedia/util/UriUtil.java
b/app/src/main/java/org/wikipedia/util/UriUtil.java
index 4157026..1467957 100644
--- a/app/src/main/java/org/wikipedia/util/UriUtil.java
+++ b/app/src/main/java/org/wikipedia/util/UriUtil.java
@@ -8,6 +8,7 @@
import android.net.Uri;
import android.support.annotation.NonNull;
import android.support.annotation.StringRes;
+import android.support.annotation.VisibleForTesting;
import android.text.TextUtils;
import android.util.Log;
@@ -120,10 +121,37 @@
return title.getCanonicalUri() + "?wprov=" + context.getString(provId);
}
- public static String removeInternalLinkPrefix(String link) {
+ /**
+ * Note that while this method also replaces '_' with spaces it doesn't
fully decode the string.
+ */
+ @NonNull
+ public static String getTitleFromUrl(@NonNull String url) {
+ return removeFragment(removeLinkPrefix(url)).replace("_", " ");
+ }
+
+ /** For internal links only */
+ @NonNull
+ public static String removeInternalLinkPrefix(@NonNull String link) {
return link.replaceFirst("/wiki/", "");
}
+ /** For links that could be internal or external links */
+ @NonNull
+ private static String removeLinkPrefix(@NonNull String link) {
+ return link.replaceFirst("^.*?/wiki/", "");
+ }
+
+ /** Removes an optional fragment portion of a URL */
+ @VisibleForTesting
+ @NonNull
+ static String removeFragment(@NonNull String link) {
+ return link.replaceFirst("#.*$", "");
+ }
+
+ public static String getFragment(String link) {
+ return Uri.parse(link).getFragment();
+ }
+
private UriUtil() {
}
diff --git a/app/src/main/java/org/wikipedia/wiktionary/WiktionaryDialog.java
b/app/src/main/java/org/wikipedia/wiktionary/WiktionaryDialog.java
index 855f22e..4616c7c 100644
--- a/app/src/main/java/org/wikipedia/wiktionary/WiktionaryDialog.java
+++ b/app/src/main/java/org/wikipedia/wiktionary/WiktionaryDialog.java
@@ -204,7 +204,7 @@
private LinkMovementMethodExt linkMovementMethod =
new LinkMovementMethodExt(new LinkMovementMethodExt.UrlHandler() {
@Override
- public void onUrlClick(String url) {
+ public void onUrlClick(@NonNull String url, @Nullable String
notUsed) {
if (url.startsWith(PATH_WIKI) ||
url.startsWith(PATH_CURRENT)) {
dismiss();
showNewDialogForLink(url);
diff --git a/app/src/test/java/org/wikipedia/util/UriUtilTest.java
b/app/src/test/java/org/wikipedia/util/UriUtilTest.java
new file mode 100644
index 0000000..54da52f
--- /dev/null
+++ b/app/src/test/java/org/wikipedia/util/UriUtilTest.java
@@ -0,0 +1,41 @@
+package org.wikipedia.util;
+
+import org.wikipedia.test.TestRunner;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+@RunWith(TestRunner.class)
+public class UriUtilTest {
+ /**
+ * Inspired by
+ * curl -s
https://en.wikipedia.org/w/api.php?action=query&meta=siteinfo&format=json&siprop=general
| jq .query.general.legaltitlechars
+ */
+ private static final String TITLE
+ = " %!\"$&'()*,\\-.\\/0-9:;=?@A-Z\\\\^_`a-z~\\x80-\\xFF+";
+
+ /**
+ * Inspired by
+ *from
http://stackoverflow.com/questions/2849756/list-of-valid-characters-for-the-fragment-identifier-in-an-url
+ */
+ private static final String LEGAL_FRAGMENT_CHARS
+ = "!$&'()*+,;=-._~:@/?abc0123456789%D8%f6";
+
+ @Test
+ public void testRemoveFragment() {
+ assertThat(UriUtil.removeFragment(TITLE + "#" + LEGAL_FRAGMENT_CHARS),
is(TITLE));
+ }
+
+ @Test
+ public void testRemoveEmptyFragment() {
+ assertThat(UriUtil.removeFragment(TITLE + "#"), is(TITLE));
+ }
+
+ @Test
+ public void testRemoveFragmentWithHash() {
+ assertThat(UriUtil.removeFragment(TITLE + "##"), is(TITLE));
+ }
+}
diff --git a/www/js/actions.js b/www/js/actions.js
index cb372e1..d6556c3 100644
--- a/www/js/actions.js
+++ b/www/js/actions.js
@@ -38,6 +38,19 @@
}
}
+/**
+ * Either gets the title from the title attribute (for mobileview case and
newer MCS pages) or,
+ * if that doesn't not exists try to derive it from the href attribute value.
+ * In the latter case it also unescapes HTML entities to get the correct title
string.
+ */
+function getTitle( sourceNode, href ) {
+ if (sourceNode.hasAttribute( "title" )) {
+ return sourceNode.getAttribute( "title" );
+ } else {
+ return href.replace(/^\/wiki\//, '').replace(/^\.\//,
'').replace(/#.*$/, '');
+ }
+}
+
document.onclick = function() {
var sourceNode = null;
var curNode = event.target;
@@ -68,7 +81,7 @@
} else if (sourceNode.classList.contains( 'image' )) {
bridge.sendMessage( 'imageClicked', { "href": href } );
} else {
- bridge.sendMessage( 'linkClicked', { "href": href } );
+ bridge.sendMessage( 'linkClicked', { "href": href, "title":
getTitle(sourceNode, href) } );
}
event.preventDefault();
}
--
To view, visit https://gerrit.wikimedia.org/r/303942
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iedbe1ae611f4d433d4c6bcac3cce3935333753e0
Gerrit-PatchSet: 16
Gerrit-Project: apps/android/wikipedia
Gerrit-Branch: master
Gerrit-Owner: BearND <bsitzm...@wikimedia.org>
Gerrit-Reviewer: BearND <bsitzm...@wikimedia.org>
Gerrit-Reviewer: Brion VIBBER <br...@wikimedia.org>
Gerrit-Reviewer: Dbrant <dbr...@wikimedia.org>
Gerrit-Reviewer: GWicke <gwi...@wikimedia.org>
Gerrit-Reviewer: Mholloway <mhollo...@wikimedia.org>
Gerrit-Reviewer: Niedzielski <sniedziel...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
