New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 596410 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug

Blocking:
issue 584303



Sign in to add a comment

[tracking bug] Add favicon and publisher canonical name to cards

Project Member Reported by maybelle@chromium.org, Mar 21 2016

Issue description

Cards don't have the favicon and publisher name on the cards.

We also need to account for situations when this data is not available, and have a default.

For favicons specifically we can fall back to our favicon fetching service and otherwise have a static drawable. Talk to maybelle@ or rachelis@ to get this drawable.
 
Labels: -Pri-3 Pri-2
Owner: mcwilliams@chromium.org
Status: Assigned (was: Untriaged)
NTPSnippets.cc also needs to be updated to parse the publisher name and favicon.

In the json, it looks like:
"sourceName": "Los Angeles Times",                                      
"sourceLogoUrl": "" 

Comment 4 by finkm@google.com, Apr 1 2016

Labels: zine-mr-mile-MVP
Cc: nepper@chromium.org
Patrick, any update on the requirements for this regarding permissions to use this data.

Thanks
Components: UI>Browser>NewTabPage
In the case of ChromeReader not having favicons, we should anyway fall back to our own favicon service so I think it's still worth doing that piece while we wait for decision on using ChromeReader's data.
Labels: OS-Android
Cc: mcwilliams@chromium.org
Owner: maybelle@chromium.org
Labels: zine-mr-iter-10 zine-mr-iter-9
Code is almost ready for review, but may spill over into next week.
Labels: zine-mr-iter-11
Ran into complications with actually getting usable favicons. Still talking to the relevant teams to see if we can use a different Google service to fetch the favicon.
Issue 605235 has been merged into this issue.
Labels: -Pri-2 Pri-1
Clean-up - obviously an MVP P1.
Labels: zine-mr-iter-12
Writing tests today, part I (without downloading the favicons from an external source) will be in CR shortly.
Labels: zine-mr-iter-13
In CR now
Project Member

Comment 17 by bugdroid1@chromium.org, May 4 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f7360bf8adcd2455a07dbf39f41f33b6bc194899

commit f7360bf8adcd2455a07dbf39f41f33b6bc194899
Author: maybelle <maybelle@chromium.org>
Date: Wed May 04 13:31:02 2016

Add favicon and publisher name to snippet cards

BUG= 596410 

Review-Url: https://codereview.chromium.org/1921553004
Cr-Commit-Position: refs/heads/master@{#391493}

[modify] https://crrev.com/f7360bf8adcd2455a07dbf39f41f33b6bc194899/chrome/android/java/res/layout/new_tab_page_snippets_card.xml
[modify] https://crrev.com/f7360bf8adcd2455a07dbf39f41f33b6bc194899/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticle.java
[modify] https://crrev.com/f7360bf8adcd2455a07dbf39f41f33b6bc194899/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetArticleViewHolder.java
[modify] https://crrev.com/f7360bf8adcd2455a07dbf39f41f33b6bc194899/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsBridge.java
[modify] https://crrev.com/f7360bf8adcd2455a07dbf39f41f33b6bc194899/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java
[modify] https://crrev.com/f7360bf8adcd2455a07dbf39f41f33b6bc194899/chrome/browser/android/ntp/ntp_snippets_bridge.cc
[modify] https://crrev.com/f7360bf8adcd2455a07dbf39f41f33b6bc194899/chrome/browser/ui/webui/snippets_internals_message_handler.cc
[modify] https://crrev.com/f7360bf8adcd2455a07dbf39f41f33b6bc194899/components/ntp_snippets/ntp_snippet.cc
[modify] https://crrev.com/f7360bf8adcd2455a07dbf39f41f33b6bc194899/components/ntp_snippets/ntp_snippet.h
[modify] https://crrev.com/f7360bf8adcd2455a07dbf39f41f33b6bc194899/components/ntp_snippets/ntp_snippets_service.cc
[modify] https://crrev.com/f7360bf8adcd2455a07dbf39f41f33b6bc194899/components/ntp_snippets/ntp_snippets_service_unittest.cc
[modify] https://crrev.com/f7360bf8adcd2455a07dbf39f41f33b6bc194899/components/ntp_snippets/switches.cc
[modify] https://crrev.com/f7360bf8adcd2455a07dbf39f41f33b6bc194899/components/ntp_snippets/switches.h
[modify] https://crrev.com/f7360bf8adcd2455a07dbf39f41f33b6bc194899/tools/metrics/histograms/histograms.xml

Status: Fixed (was: Assigned)
Labels: zine-mr-MVP
Labels: -zine-mr-mvp
Labels: zine-mr-MVP

Sign in to add a comment