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

Issue 736254 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocked on:
issue 741340



Sign in to add a comment

no NTP icon (gray icon) for HRS

Project Member Reported by tschumann@chromium.org, Jun 23 2017

Issue description

Happens on both dev (topsites) and stable (most likely).
For HRS, I see gray NTP icons (even after clicking it).

On Dev:
From about://ntp-tiles-internals:
Source: TOP_SITES
URL: http://www.hrs.de
Note, that this page redirects to http://touch.hrs.de


On Stable:
From about://ntp-tiles-internals:
Source: SUGGESTIONS_SERVICE
URL: https://touch.hrs.de


Attaching the <head> element with more details about the touch.hrs.de page:

<head> <title>HRS | hotel booking quick and safe at best prices</title> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> <meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=0"> <meta name="format-detection" content="telephone=no"> <meta name="msapplication-tap-highlight" content="no"> <link rel="canonical" href="http://www.hrs.de"> <link rel="alternate" hreflang="de" href="http://touch.hrs.de/?lang=de"> <link rel="alternate" hreflang="en" href="http://touch.hrs.de/?lang=en"> <link rel="alternate" hreflang="fr" href="http://touch.hrs.de/?lang=fr"> <link rel="alternate" hreflang="it" href="http://touch.hrs.de/?lang=it"> <link rel="alternate" hreflang="es" href="http://touch.hrs.de/?lang=es"> <link rel="alternate" hreflang="zh" href="http://touch.hrs.de/?lang=zh"> <link rel="alternate" hreflang="pl" href="http://touch.hrs.de/?lang=pl"> <link rel="alternate" hreflang="ru" href="http://touch.hrs.de/?lang=ru"> <link rel="alternate" hreflang="ko" href="http://touch.hrs.de/?lang=ko"> <link rel="alternate" hreflang="ja" href="http://touch.hrs.de/?lang=ja"> <link rel="alternate" hreflang="tr" href="http://touch.hrs.de/?lang=tr"> <link rel="alternate" hreflang="nl" href="http://touch.hrs.de/?lang=nl"> <meta name="mobile-web-app-capable" content="yes"> <link rel="apple-touch-icon" href="hrs-touch-icon.png"> <meta name="title" content="HRS | hotel booking quick and safe at best prices"> <meta name="description" content="Easy hotel search and booking on mobile devices | Over 300.000 hotels worldwide at best prices!"> <meta name="google" value="notranslate" content="notranslate"> <link rel="manifest" href="./manifest.json"> <meta name="application-name" content="HRS"> <link rel="shortcut icon" href="favicon.ico" type="image/x-icon"> <link rel="stylesheet" type="text/css" href="webkit.css?v=r-4.19.0"> <script async="true" type="text/javascript" src="https://sslwidget.criteo.com/event?a=1334&amp;v=4.1.0&amp;p0=e%3Dexd%26site_type%3Dm&amp;p1=e%3Dvh&amp;p2=e%3Ddis&amp;adce=1" data-owner="criteo-tag"></script><script type="text/javascript" src="https://sjs.bizographics.com/insight.min.js"></script><script src="https://connect.facebook.net/signals/config/513769908825072?v=2.7.16" async=""></script><script type="text/javascript" async="" charset="utf-8" id="utag_1383" src="https://connect.facebook.net/en_US/fbevents.js"></script><script type="text/javascript" async="" charset="utf-8" id="utag_1235" src="//static.criteo.net/js/ld/ld.js"></script><script type="text/javascript" async="" charset="utf-8" id="utag_1226" src="//www.googleadservices.com/pagead/conversion_async.js"></script><script type="text/javascript" async="" charset="utf-8" id="utag_1310" src="//tags.tiqcdn.com/utag/tiqapp/utag.currency.js"></script><script type="text/javascript" async="" src="https://www.google-analytics.com/analytics.js"></script><script async="" src="//www.googletagmanager.com/gtm.js?id=GTM-W8NDHF"></script><script type="text/javascript" src="initial.js?v=r-4.19.0"></script> <script type="text/javascript" src="pf.js?v=r-4.19.0"></script> <script type="text/javascript" src="pfe.js?v=r-4.19.0"></script> <meta name="apple-itunes-app" content="app-id=332193586, app-argument=hrsapp://start"> <script type="text/javascript" async="" charset="utf-8" id="utag_hrs.touchweb_1310" src="//tags.tiqcdn.com/utag/hrs/touchweb/prod/utag.1310.js?utv=ut4.42.201610241009"></script><script type="text/javascript" async="" charset="utf-8" id="utag_hrs.touchweb_1206" src="//tags.tiqcdn.com/utag/hrs/touchweb/prod/utag.1206.js?utv=ut4.42.201609081221"></script><script type="text/javascript" async="" charset="utf-8" id="utag_hrs.touchweb_1226" src="//tags.tiqcdn.com/utag/hrs/touchweb/prod/utag.1226.js?utv=ut4.42.201706011207"></script><script type="text/javascript" async="" charset="utf-8" id="utag_hrs.touchweb_1235" src="//tags.tiqcdn.com/utag/hrs/touchweb/prod/utag.1235.js?utv=ut4.42.201703301428"></script><script type="text/javascript" async="" charset="utf-8" id="utag_hrs.touchweb_1383" src="//tags.tiqcdn.com/utag/hrs/touchweb/prod/utag.1383.js?utv=ut4.42.201706011207"></script><script type="text/javascript" async="" charset="utf-8" id="utag_hrs.touchweb_1485" src="//tags.tiqcdn.com/utag/hrs/touchweb/prod/utag.1485.js?utv=ut4.42.201706011207"></script><script id="undefined" type="text/javascript" src="//track.adform.net/serving/scripts/trackpoint/async/"></script><script src="https://track.adform.net/Serving/TrackPoint/?pm=251477&amp;ADFPageName=Mobile%20Web%7CHomepage&amp;ADFdivider=%7C&amp;ord=468769003922&amp;Set1=en-US%7Cen-US%7C2560x1600%7C24&amp;ADFtpmode=2&amp;itm=eyJzdjEzIjoiTW9iaWxlJTIwV2ViJTdDSG9tZXBhZ2UiLCJzdjkiOiIxMDUyNTU2MDAxIn0&amp;loc=https%3A%2F%2Ftouch.hrs.de%2F" async=""></script></head>
 

Comment 1 by mastiz@chromium.org, Jun 23 2017

Thanks for reporting. Client-side redirects are pretty tricky and poorly handled.

I happen to be working on an idea that could fix this case: https://codereview.chromium.org/2949023002/

I'll get back to this issue once I have a better idea about what the right fix is, and verify if it does indeed solve the problem.

Comment 2 by mastiz@chromium.org, Jun 23 2017

Labels: zine-favicon-pe
https://codereview.chromium.org/2949023002/ does indeed fix this issue.

Comment 4 by mastiz@chromium.org, Jul 12 2017

Blockedon: 741340

Comment 5 by fi...@chromium.org, Jul 21 2017

Labels: zine-triaged

Comment 6 by mastiz@chromium.org, Jul 26 2017

Status: Started (was: Untriaged)
One related CL has landed and a second one, that would fix this issue, is under review: https://chromium-review.googlesource.com/c/585130/
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 15 2017

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

commit 047511efacd04f9849483a1b9060927e97d05b95
Author: Mikel Astiz <mastiz@chromium.org>
Date: Tue Aug 15 22:23:32 2017

Propagate favicons across redirect-like pushState/replaceState

Many pages use Javascript to change the URL in the omnibox with the
history API, namely pushState() and replaceState(), which are considered
in-page navigations because the actual content of the page doesn't
change.

For pages that trigger such functions early, before the page has loaded,
no favicon candidates are received (because content reports favicons in
a late stage) and hence no favicons are associated to the URL prior to
the URL update. This leads to gray tiles in the New Tab Page, because
they point to the first.

We fix this by special-handling this particular scenario, by introducing
a state variable (FaviconHandler::candidates_received_) to distinguish
whether the previous page was fully loaded (or at least candidates were
provided).

Bug:  741340 , 736254 , 754386 
Change-Id: I2c11a3238bf1e14652bb20abc8cfbb3c46dc848a
Reviewed-on: https://chromium-review.googlesource.com/595977
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494574}
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/chrome/browser/favicon/content_favicon_driver_browsertest.cc
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/chrome/browser/sessions/session_restore_delegate.cc
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/chrome/browser/sessions/tab_loader.cc
[add] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/chrome/test/data/favicon/pushstate_with_favicon.html
[add] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/chrome/test/data/favicon/replacestate_with_favicon.html
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/components/favicon/content/content_favicon_driver.cc
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/components/favicon/core/favicon_driver.h
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/components/favicon/core/favicon_driver_impl.cc
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/components/favicon/core/favicon_driver_impl.h
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/components/favicon/core/favicon_handler.cc
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/components/favicon/core/favicon_handler.h
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/components/favicon/core/favicon_handler_unittest.cc
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/components/favicon/core/favicon_service.h
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/components/favicon/core/favicon_service_impl.cc
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/components/favicon/core/favicon_service_impl.h
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/components/favicon/core/test/mock_favicon_service.h
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/components/favicon/ios/web_favicon_driver.h
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/components/favicon/ios/web_favicon_driver.mm
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/components/history/core/browser/history_backend.cc
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/components/history/core/browser/history_backend.h
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/components/history/core/browser/history_backend_unittest.cc
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/components/history/core/browser/history_service.cc
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/components/history/core/browser/history_service.h
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/ios/chrome/browser/reading_list/reading_list_distiller_page.mm
[modify] https://crrev.com/047511efacd04f9849483a1b9060927e97d05b95/ios/chrome/browser/tabs/tab.mm

Status: Fixed (was: Started)
Verified on dev.

Sign in to add a comment