chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java - Issue 2342453003: [NTP] Fix article suggestion clicks contributing to Most Visited tiles

Unified Diff: chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java

Issue 2342453003: [NTP] Fix article suggestion clicks contributing to Most Visited tiles (Closed)

Patch Set: Created 4 years, 3 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

« no previous file with comments | « chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java ('k') | chrome/android/java_sources.gni » ('j') | components/history/core/browser/history_backend.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java

index 7fb037243e4133847d51008b3c1b771c26403b64..3379fde0d338f427aa5d50258ed2125115f97f77 100644

--- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java

+++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java

@@ -181,4 +181,40 @@ public class SnippetArticle implements NewTabPageItem {

int recordedScore = Math.min((int) Math.ceil(score), 100000);

RecordHistogram.recordCustomCountHistogram(histogramName, recordedScore, 1, 100000, 50);

}

+ /**

+ * Construct a data:text/html URI for loading from an inline HTML. Copied

+ * from org.chromium.base.test.util.UrlUtils.

+ * @param html An unencoded HTML

+ * @return String An URI that contains the given HTML

+ */

+ private static String encodeHtmlDataUri(String html) {

+ try {

+ // URLEncoder encodes into application/x-www-form-encoded, so

+ // ' '->'+' needs to be undone and replaced with ' '->'%20'

+ // to match the Data URI requirements.

+ String encoded = "data:text/html;utf-8," + java.net.URLEncoder.encode(html, "UTF-8");

+ encoded = encoded.replace("+", "%20");

+ return encoded;

+ } catch (java.io.UnsupportedEncodingException e) {

+ return null;

+ }

+ /**

+ * Returns the URL that should actually be opened, which is either the

+ * original URL associated to this article or a redirect implemented via

+ * data: scheme (which prevents articles from showing up in NTP tiles.

Marc Treib 2016/09/14 13:25:14 nit: missing closing paren

mastiz 2016/09/14 13:46:21 Done.

+ */

+ public String getPossiblyWrappedUrl() {

+ if (mCategory != KnownCategories.ARTICLES) return mUrl;

+ // Baked in HTML page that causes a redirect to mURL. Note that mUrl

+ // is expected to specify a scheme for the redirect to work (guaranteed

+ // because it's coming from a GURL).

+ String wrappedUrl = encodeHtmlDataUri(

+ "<head><meta http-equiv=\"refresh\" content=\"0; url=" + mUrl + "\"></head>");

+ if (wrappedUrl == null) return mUrl;

Marc Treib 2016/09/14 13:25:14 Under what circumstances would we expect this to h

mastiz 2016/09/14 13:46:21 I would expect this to never happen since we're en

Marc Treib 2016/09/14 13:47:55 SGTM for merging, but then please add a TODO to ch

Bernhard Bauer 2016/09/14 13:53:08 Yeah, I wouldn't expect UTF-8 encoding to fail, so

mastiz 2016/09/14 15:42:51 Added TODO for now.

+ return wrappedUrl;

+ }

}